diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 1aa47c6f..752ae8d2 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -8,10 +8,10 @@ { "name": "bcquality", "source": "./", - "description": "Business Central AL quality knowledge base and review skills, packaged as an installable plugin. Ships the entire BCQuality tree (skills, knowledge, tools) so the Entry routing protocol runs against the installed clone.", - "version": "0.1.0", + "description": "Business Central AL quality knowledge base and review skills, packaged as an installable plugin. Exposes an AL review adapter while preserving BCQuality's internal Entry and action-skill protocols.", + "version": "0.2.0", "skills": [ - "./skills/bcquality-al-review/" + "./skills/" ] } ] diff --git a/.github/scripts/validate_frontmatter.py b/.github/scripts/validate_frontmatter.py index dd3ef4ed..b0076cce 100644 --- a/.github/scripts/validate_frontmatter.py +++ b/.github/scripts/validate_frontmatter.py @@ -42,6 +42,7 @@ } META_SKILL_REQUIRED_KEYS = {"kind", "id", "version", "title"} ENTRY_SKILL_REQUIRED_KEYS = {"kind", "id", "version", "title"} +HOST_SKILL_REQUIRED_KEYS = {"name", "description"} STANDARD_INPUTS = { "pr-diff", "object-list", "file-path", "repository", "telemetry-query", @@ -444,10 +445,36 @@ def validate_entry_skill(path: Path, parsed: Parsed, report: Report) -> None: report.error(path, "R23", f"version must be a positive integer: {v!r}", 1) +def validate_host_skill(path: Path, parsed: Parsed, report: Report) -> None: + if parsed.frontmatter_error: + report.error(path, "R01", parsed.frontmatter_error, 1) + return + fm = parsed.frontmatter + assert fm is not None + missing = HOST_SKILL_REQUIRED_KEYS - fm.keys() + if missing: + report.error(path, "R29", f"missing required host-skill keys: {sorted(missing)}", 1) + + name = fm.get("name") + if not isinstance(name, str) or not name: + report.error(path, "R29", "host-skill name must be a non-empty string", 1) + else: + if len(name) > 64 or not KEBAB_CASE.fullmatch(name): + report.error(path, "R29", f"host-skill name must be lowercase kebab-case and at most 64 characters: '{name}'", 1) + if name != path.parent.name: + report.error(path, "R29", f"host-skill name must match parent directory '{path.parent.name}', got '{name}'", 1) + + description = fm.get("description") + if not isinstance(description, str) or not description: + report.error(path, "R29", "host-skill description must be a non-empty string", 1) + elif len(description) > 1024: + report.error(path, "R29", "host-skill description must be at most 1024 characters", 1) + + # --- Path and sample checks ------------------------------------------------- def classify(path_from_root: Path) -> str | None: - """Return 'knowledge' | 'action-skill' | 'meta' | 'entry' | None.""" + """Return 'knowledge' | 'action-skill' | 'host-skill' | 'meta' | 'entry' | None.""" parts = path_from_root.parts if len(parts) < 2: return None @@ -459,6 +486,8 @@ def classify(path_from_root: Path) -> str | None: return "entry" if name in META_SKILL_FILES: return "meta" + if len(parts) == 3 and parts[2] == "SKILL.md": + return "host-skill" return None if top in LAYERS and path_from_root.suffix == ".md": if len(parts) >= 3 and parts[1] == "skills": @@ -616,6 +645,8 @@ def run(root: Path) -> Report: validate_entry_skill(path, parsed, report) if parsed.frontmatter and isinstance(parsed.frontmatter.get("id"), str): skill_records.append(SkillRecord(path, "entry-point", parsed.frontmatter["id"])) + elif kind == "host-skill": + validate_host_skill(path, parsed, report) # Second pass: sample files per knowledge domain for layer in LAYERS: diff --git a/README.md b/README.md index 8946d9c9..7933725e 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,52 @@ Skills define how agents consume knowledge. They come in three flavors: An orchestrator (such as AL-Go) points the agent at BCQuality's URL and provides a task context. The agent's first call is `/skills/entry.md`, which returns a dispatch record naming the action skill(s) to invoke. The agent then invokes each dispatched skill in turn, reading READ and DO on demand. No prior knowledge of BCQuality's structure is baked into the orchestrator — only the convention *"invoke `/skills/entry.md` first."* +### Standalone plugin installation + +BCQuality can also be installed directly as a plugin. The plugin registers one +host-native skill, +[`al-code-review`](skills/al-code-review/SKILL.md), which adapts the caller's +request to the same Entry protocol used by orchestrators. + +For GitHub Copilot CLI: + +```shell +copilot plugin install microsoft/BCQuality +``` + +Plugin version `0.2.0` renamed the former `bcquality-al-review` skill to +`al-code-review`; explicit invocations and allowlists using the old skill name +must be updated. The name remains distinct from BC-ALAgents' public +`al-review` skill because current hosts may load plugin skill names into one +shared inventory. + +The adapter is intentionally not a second review 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 first file follows the host's `SKILL.md` packaging format. The +remaining files are BCQuality's internal protocol and layered action skills. +Entry remains the single owner of routing and index preparation; +`al-code-review.md` remains the single owner of broad-review composition. This +separation keeps standalone installation available without duplicating those +policies in the plugin adapter. + +Note that a plugin install ships the entire tree, so `BCQUALITY_ENABLED_LAYERS` +narrows discovery without removing any files. Layer selection is a filter here, +not a deny mechanism — see [the adapter](skills/al-code-review/SKILL.md) for the +difference from the pruned-clone model. + +The host adapter and internal action skill intentionally share the +`al-code-review` name: they expose the same operation in two different skill +formats. Their paths make the boundary explicit. The adapter lives under +`skills/al-code-review/SKILL.md`; the internal Microsoft-layer coordinator +lives at `microsoft/skills/review/al-code-review.md`. + ## Knowledge file format Every knowledge file is a markdown file with mandatory YAML frontmatter. Files target under 100 lines (ideal under 50). If two ideas would share a file, split them. diff --git a/agent-consumption.md b/agent-consumption.md index 8db80d65..61eb3c9e 100644 --- a/agent-consumption.md +++ b/agent-consumption.md @@ -12,6 +12,10 @@ For the high-level framing and repo structure, start with the [README](README.md - **Global skills** in `/skills/` — the `entry.md` entry-point skill plus the READ · DO · WRITE contracts that govern the rest of the repo. - **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. + ## The flow ```mermaid @@ -31,6 +35,12 @@ 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 +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. @@ -89,6 +99,10 @@ Orchestrators MUST tolerate an absent `domain` in reports from older producers. ## Why this architecture - **Entry is the only hardcoded thing.** Orchestrators ship with one convention — *"invoke `/skills/entry.md` first"* — and nothing else. New action skills and new knowledge files are picked up automatically because Entry discovers them at dispatch time. +- **Standalone installation adds an adapter, not another policy layer.** The + plugin's host-format `al-code-review` skill only translates the invocation + into Entry's task context. Entry and the dispatched action skills remain + authoritative. - **Layers decide authority, not code.** The agent sees `/microsoft/` and `/community/` together; if two files conflict, the precedence rule defined in READ resolves it. A partner fork can disable `/community/` — that's a config choice, not a code change. - **Knowledge and skills evolve independently.** A new knowledge file requires no skill changes — existing skills pick it up via frontmatter filters. A new skill requires no knowledge changes — it sources from what's already there. diff --git a/plugin.json b/plugin.json index 0934bd34..c99785b0 100644 --- a/plugin.json +++ b/plugin.json @@ -1,7 +1,7 @@ { "name": "bcquality", - "description": "Quality skills and knowledge for Business Central development. Exposes a review bridge skill that drives the BCQuality Entry protocol over the installed knowledge base.", - "version": "0.1.0", + "description": "Quality skills and knowledge for Business Central development. Exposes a standalone AL review adapter backed by BCQuality's Entry protocol.", + "version": "0.2.0", "author": { "name": "microsoft/BCQuality", "url": "https://github.com/microsoft/BCQuality" @@ -16,6 +16,6 @@ "quality" ], "skills": [ - "./skills/bcquality-al-review/" + "./skills/" ] } diff --git a/skills/README.md b/skills/README.md index d0500a3e..3d8fdfb5 100644 --- a/skills/README.md +++ b/skills/README.md @@ -1,6 +1,9 @@ # BCQuality global skills -This folder contains the skills that are not owned by any single layer. There are two kinds: +This folder contains BCQuality's layer-independent protocol files and the +host-native adapter used by standalone plugin installations. + +The protocol files have two kinds: - **The entry-point skill** — the first skill an agent invokes at runtime. - **The three meta-skill contracts** — stable references that define what the rest of BCQuality means. @@ -23,6 +26,35 @@ Routing logic lives in Entry, not in the orchestrator. An agent that knows only READ and DO are read on demand — typically by the first action skill the agent executes after dispatch. They are not prerequisites for invoking Entry. WRITE is only used when scaffolding new content. +## Standalone plugin adapter + +| Path | Role | +|---|---| +| [`al-code-review/SKILL.md`](al-code-review/SKILL.md) | Exposes BCQuality through the standard `SKILL.md` format when this repository is installed as a plugin. | + +The adapter is deliberately thin. It translates the caller's request into an +Entry task context, then follows Entry's dispatch without owning routing, +review, index, or output policy. It is not an action skill, is not considered +by Entry, and should not accumulate behavior already defined by `entry.md`, +`read.md`, `do.md`, or a layered action skill. + +This gives the two skill formats distinct roles: + +- `skills/al-code-review/SKILL.md` is the public host integration surface for a + standalone plugin installation. +- `microsoft/skills/review/al-code-review.md` is BCQuality's internal + Microsoft-layer super-skill for coordinating a broad AL review. + +The host adapter and internal coordinator deliberately share the +`al-code-review` name because they represent the same user-facing operation in +their respective formats. Their locations distinguish their roles. The +adapter remains distinct from BC-ALAgents' separately installed `al-review` +skill, avoiding a collision in hosts that use one shared skill inventory. The +reference from the adapter to Entry, and from a dispatched super-skill to its +leaf skills, is intentional progressive disclosure. It avoids registering +every internal BCQuality protocol file as an ambient host skill while allowing +each review domain to run in an isolated context. + These contracts are stable. Changes require a PR approved by both maintainers. For the end-to-end flow — from orchestrator trigger through to findings integration — see [`../agent-consumption.md`](../agent-consumption.md). For the high-level project framing, see [`../README.md`](../README.md). diff --git a/skills/al-code-review/SKILL.md b/skills/al-code-review/SKILL.md new file mode 100644 index 00000000..991a771d --- /dev/null +++ b/skills/al-code-review/SKILL.md @@ -0,0 +1,69 @@ +--- +name: al-code-review +description: Review Business Central AL code changes using BCQuality's curated rules. Use for an AL pull request, working-tree diff, branch, or individual AL file when BCQuality is installed as a standalone plugin. +--- + +# AL code review + +This is BCQuality's host-native adapter for standalone plugin installations. It +is not a BCQuality action skill and contains no review or routing policy. Its +only responsibility is to translate the caller's request into an Entry task +context and execute the resulting dispatch. + +## Execute + +1. Resolve `PLUGIN_ROOT` to the directory containing this plugin's root + `plugin.json`. This file is + `PLUGIN_ROOT/skills/al-code-review/SKILL.md`; when the host does not expose + the plugin root, resolve it two levels above this file. +2. Build the `task-context` required by + `PLUGIN_ROOT/skills/entry.md`: + - Copy the caller's actual request verbatim into `goal`; do not replace a + focused request such as "review performance" with a generic full-review + goal. + - Set `inputs-available` to the inputs actually available to the review, + normally `pr-diff` for changes or `file-path` for one file. + - Set `technologies: [al]` when the input is known to be AL. + - Pass `bc-version`, `countries`, and `application-area` only when supplied + or reliably determined. + - If `BCQUALITY_ENABLED_LAYERS` is set, split its comma-separated value and + pass the trimmed, non-empty entries as `enabled-layers`; otherwise omit the + field and let Entry apply its default. + - If `BCQUALITY_DISABLED_SKILLS` is set, split its comma-separated value and + pass the trimmed, non-empty entries as `disabled-skills`; otherwise omit + the field. +3. Read and execute `PLUGIN_ROOT/skills/entry.md` exactly as written, including + its Preparation step. Entry is authoritative for index freshness, routing, + defaults, and failure behavior; this adapter must not duplicate or weaken + those rules. Entry is written for a checkout whose root is the current + directory, so resolve every repo-relative path it names against + `PLUGIN_ROOT` rather than the caller's working directory, which is the + user's own project. In particular, run Preparation's index build as + `pwsh PLUGIN_ROOT/tools/Build-KnowledgeIndex.ps1`: the generator resolves + its own root and writes `PLUGIN_ROOT/knowledge-index.json`, which is not + shipped and is therefore absent on a fresh install. If `pwsh` is + unavailable or the build fails, continue — READ falls back to path-based + discovery — but do not treat a failed build as a failed review. +4. Follow Entry's **How the agent uses the dispatch** instructions. Invoke only + the returned action skills, pass each dispatch entry's exact input subset, + and read `PLUGIN_ROOT/skills/read.md` and `PLUGIN_ROOT/skills/do.md` on + demand. When a dispatched super-skill requests isolated leaf execution and + the host supports child contexts, use them. +5. Return each dispatched action skill's findings report unchanged. If Entry + returns `no-match` or `failed`, return its dispatch record unchanged. + +The internal `microsoft/skills/review/al-code-review.md` action skill remains +the canonical coordinator for a broad AL review. Entry decides whether that +super-skill or a narrower domain skill applies; this host adapter never chooses +between them. + +## Layer selection is not a deny mechanism + +A plugin install ships the whole BCQuality tree, so `enabled-layers` here can +only narrow *discovery*: the files of a layer left out of the list still exist +on disk. This differs from the clone model Entry's Preparation step describes, +where a consumer prunes its checkout to policy before the agent runs and the +index is rebuilt over the pruned tree. Treat `BCQUALITY_ENABLED_LAYERS` as a +selection filter, never as a security boundary. A host that needs a genuine +deny mechanism must prune the installed tree itself. + diff --git a/skills/bcquality-al-review/SKILL.md b/skills/bcquality-al-review/SKILL.md deleted file mode 100644 index 32c82072..00000000 --- a/skills/bcquality-al-review/SKILL.md +++ /dev/null @@ -1,100 +0,0 @@ ---- -name: bcquality-al-review -description: Review Business Central AL code changes using the BCQuality knowledge base. Use when reviewing an AL pull request, a working-tree diff, or a single AL file, and you want findings backed by BCQuality's curated, BC-specific quality rules. ---- - -# BCQuality AL review - -This skill drives the BCQuality **Entry protocol** over the knowledge base that ships -inside this plugin. It is the plugin entry point for consumers (orchestrators, CLIs) -that do not already know BCQuality's internal conventions — the only convention they -need is "invoke this skill for an AL review." - -BCQuality itself is orchestrator-agnostic content: knowledge files plus routing and -action skills. This bridge is the thin consumer glue that lets a plugin host run that -content without hardcoding BCQuality's layout. - -## When to use - -- Reviewing an AL pull request or an uncommitted working-tree diff. -- Reviewing a single AL file. -- Any task whose goal is "review Business Central / AL code for quality issues." - -Do **not** use this skill to *generate* AL code — it only reviews. - -## Plugin root - -Resolve `PLUGIN_ROOT` to the directory that contains this plugin's root -`plugin.json`. This skill lives at -`PLUGIN_ROOT/skills/bcquality-al-review/SKILL.md`, so `PLUGIN_ROOT` is two levels up -from this file. All paths below are relative to `PLUGIN_ROOT`. If the host exposes a -plugin-root environment variable, prefer it. - -## Steps - -1. **Refresh the knowledge index (best effort).** If `pwsh` is available, run - `pwsh PLUGIN_ROOT/tools/Build-KnowledgeIndex.ps1` from `PLUGIN_ROOT` to (re)generate - `PLUGIN_ROOT/knowledge-index.json` over the installed tree. This is a discovery - accelerator only — if `pwsh` is missing or the build fails, continue; the review - skills fall back to path-based discovery. - -2. **Run Entry.** Read `PLUGIN_ROOT/skills/entry.md` and execute it against a - task context describing the review: - - ```yaml - task-context: - goal: "Review the AL changes for quality issues" - inputs-available: [pr-diff] # or [file-path] for single-file review - technologies: [al] - enabled-layers: [microsoft, community, custom] # see "Layer selection" below - ``` - - **Layer selection.** `enabled-layers` defaults to all three layers. A host can - narrow it by setting the `BCQUALITY_ENABLED_LAYERS` environment variable to a - comma-separated subset (e.g. `microsoft` or `microsoft,community`); when set, pass - exactly those layers instead of the default. This is the plugin path's only knob - for layer policy — see the limitation in Notes. - - Fill `bc-version`, `countries`, and `application-area` only when the caller - supplies them; omit them otherwise (an omitted dimension is unconstrained). - -3. **Follow the dispatch record.** Entry returns a dispatch record naming the action - skill(s) to invoke — for a PR review this is normally - `microsoft/skills/review/al-code-review.md`. For each dispatched skill, read the - file and execute its Source → Relevance → Worklist → Action steps, reading - `PLUGIN_ROOT/skills/read.md` and `PLUGIN_ROOT/skills/do.md` on demand. - When `al-code-review` composes its leaves and the host supports child contexts or - separate model calls, run each leaf in an isolated context and roll up the returned - JSON. Pass each call the exact index rows for that leaf's domain so references can - be copied verbatim. This is the preferred execution profile for fast/small models; - do not force one generation to retain all domain knowledge at once. - -4. **Emit findings.** Produce the rolled-up findings report in the DO output contract, - including each review finding's producer-supplied `domain` label (`outcome`, - `findings`, `references`, `confidence`, `suppressed`). Do not invent a different - shape; downstream consumers parse the DO contract without skill-specific logic. - Apply DO's reference-integrity gate before returning: every knowledge-backed path - must exist in the installed tree, must have been opened in full, and must be copied - verbatim. Never synthesize a plausible article slug. - -If Entry returns `no-match` or `failed`, return the dispatch record unchanged so the -caller can log the reason. - -## Notes - -- This skill adds nothing to BCQuality's knowledge or routing logic; it only bootstraps - the existing Entry protocol from a plugin host. Knowledge and skill changes belong in - the layers under `PLUGIN_ROOT/microsoft/`, `PLUGIN_ROOT/community/`, and - `PLUGIN_ROOT/custom/`, not here. -- **Layer pruning is coarser than the URL/clone model.** In the clone model a consumer - prunes its checkout to policy *before* the agent runs, and the knowledge index is - rebuilt over the pruned tree, so a denied layer can never leak into discovery. A - plugin install ships the whole tree, so this bridge can only *narrow discovery* via - `enabled-layers` (`BCQUALITY_ENABLED_LAYERS`) — the denied layers' files still exist on - disk. Treat `enabled-layers` as a selection filter, not a hard security boundary. A - future revision could add a genuine deny mechanism (e.g. pruning the installed tree). -- **Manifest location.** This plugin's manifest is the root `plugin.json`, which both - Claude Code and Copilot CLI accept (verified with Copilot CLI: `plugin install` - reports the bridge skill loaded). A `.claude-plugin/marketplace.json` alongside it - carries the marketplace entry. Claude Code also reads `.claude-plugin/plugin.json`; if - a future host only reads that form, dual-home the manifest there. diff --git a/skills/entry.md b/skills/entry.md index f094aa6d..0196c355 100644 --- a/skills/entry.md +++ b/skills/entry.md @@ -35,7 +35,7 @@ task-context: ## 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). Because 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 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: - If `knowledge-index.json` is absent — or you cannot confirm it reflects the current knowledge tree — regenerate it by running, from the checkout root: @@ -44,6 +44,8 @@ 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. +- 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. Generation is **owned by BCQuality**: the generator ships here next to the skills and knowledge it derives from, and the consuming orchestrator neither builds nor knows about the index.