From acbdd975d19eb57e5c6711c80f42cdaf5c466c4e Mon Sep 17 00:00:00 2001 From: Avery Milandin Date: Mon, 24 Aug 2026 14:56:18 -0700 Subject: [PATCH 01/10] feat(ess-maker-skills): add /harden instruction review skill Adds a `/harden` capability that reviews an agent's system instructions for internal contradictions and for the gaps that let it answer from something other than its knowledge sources, or offer actions nothing authorized. - `.github/prompts/harden.prompt.md` - setup gate + delegation - `src/skills/instructions/harden/SKILL.md` - the ten-step flow - `src/reference/ess-docs/hardening/instruction-rules.md` - generic rule pack covering contradictions, grounding gaps, over-commitment, and the symmetric over-restriction risk - `scripts/check_instruction_budget.py` - deterministic character-budget probe whose verdict the skill treats as authoritative - registers `harden` in ADK_CAPABILITIES, the menu, and the README The skill asks what the maker has actually seen before analyzing, and does not tighten instructions when nothing specific is wrong: extra prohibitions cause the agent to decline questions its sources answer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 96774230-f664-4887-baf1-a5bbfbd6cbb2 --- .../.github/prompts/harden.prompt.md | 21 ++ .../.github/prompts/menu.prompt.md | 1 + solutions/ess-maker-skills/README.md | 12 + .../ess-maker-skills/scripts/adk_telemetry.py | 2 + .../scripts/check_instruction_budget.py | 166 ++++++++++++ .../ess-docs/hardening/instruction-rules.md | 196 ++++++++++++++ .../src/skills/instructions/harden/SKILL.md | 226 ++++++++++++++++ tests/scripts/test_instruction_budget.py | 243 ++++++++++++++++++ tests/test_adk_telemetry.py | 1 + 9 files changed, 868 insertions(+) create mode 100644 solutions/ess-maker-skills/.github/prompts/harden.prompt.md create mode 100644 solutions/ess-maker-skills/scripts/check_instruction_budget.py create mode 100644 solutions/ess-maker-skills/src/reference/ess-docs/hardening/instruction-rules.md create mode 100644 solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md create mode 100644 tests/scripts/test_instruction_budget.py diff --git a/solutions/ess-maker-skills/.github/prompts/harden.prompt.md b/solutions/ess-maker-skills/.github/prompts/harden.prompt.md new file mode 100644 index 00000000..f34da9d5 --- /dev/null +++ b/solutions/ess-maker-skills/.github/prompts/harden.prompt.md @@ -0,0 +1,21 @@ +--- +mode: agent +description: "Type Enter to review and harden your agent's instructions against ungrounded or over-committing answers" +--- + +# Harden + +**Setup-state check.** Read `.local/config.json`. If it does not exist, OR `setup` is not `"complete"`, show: + +> Welcome to the ESS Maker Kit. Before running `/harden`, type `/setup` to set up your environment. + +and STOP. Otherwise proceed with the skill instructions below. + +You are helping a maker review their agent's **system instructions** — the standing guidance the agent +follows on every turn — for internal contradictions and for the gaps that let an agent answer confidently +from something other than its knowledge sources, or offer to do things it cannot do. + +Every change is **proposed, never applied silently**: the maker sees the exact before-and-after text and +approves it. + +Read the skill instructions at `src/skills/instructions/harden/SKILL.md`, then follow the steps in order. diff --git a/solutions/ess-maker-skills/.github/prompts/menu.prompt.md b/solutions/ess-maker-skills/.github/prompts/menu.prompt.md index 9cd34568..40644e6b 100644 --- a/solutions/ess-maker-skills/.github/prompts/menu.prompt.md +++ b/solutions/ess-maker-skills/.github/prompts/menu.prompt.md @@ -19,6 +19,7 @@ Here's what I can help you with: | `/delete` | Type Enter to delete a topic or workflow from your agent | | `/scan` | Type Enter to scan your agent for compile errors and fix them | | `/review` | Type Enter to review a topic (or a whole module's topics) for issues before publishing | +| `/harden` | Type Enter to review and harden your agent's instructions against ungrounded or over-committing answers | | `/test` | Type Enter to drive and debug a topic or workflow's runtime behaviour until it's right | | `/evaluate` | Type Enter to generate evaluation test sets for your agent | | `/flightcheck` | Type Enter to run a pre-deployment readiness check | diff --git a/solutions/ess-maker-skills/README.md b/solutions/ess-maker-skills/README.md index 276f8f21..a59ede87 100644 --- a/solutions/ess-maker-skills/README.md +++ b/solutions/ess-maker-skills/README.md @@ -71,6 +71,18 @@ Catch and fix compile errors before they reach production. The `/scan` command a - Proposes fixes and applies them with your confirmation - Re-scans after each fix to verify resolution +### 🛡️ Harden Agent Instructions + +Review your agent's **system instructions** — the standing guidance it follows on every turn — for the gaps that let it answer confidently from something other than its knowledge sources, or offer to do things it can't actually do. Run `/harden`. + +- **Asks what you've actually seen first** — paste an answer you didn't like and the review works backward from it to the instruction that permitted it +- **Always checks for contradictions** — a rule contradicted elsewhere in the instructions isn't in force, however firmly it's written, and this runs even when the agent is behaving well +- **Proposes line-level diffs** — exact before-and-after text with a reason, applied only after you approve; never a wholesale rewrite +- **Won't tighten for the sake of it** — if nothing is wrong, it says so. Extra prohibitions make the agent decline questions it could have answered, which is a real regression traded for a hypothetical one +- **Enforces the length ceiling** — instructions have a character limit and hardening only adds text, so the pass measures the result and proposes what comes out when it doesn't fit + +Instruction changes are behavioral changes, so `/harden` hands off to `/evaluate` and `/test` to check them — including the answers you reported, which are the only direct evidence of whether the change worked. + ### 📊 Generate Evaluation Test Sets Create structured CSV test sets that you upload to the Copilot Studio Evaluation portal. The agent reads your topics and generates tests across multiple quality dimensions. diff --git a/solutions/ess-maker-skills/scripts/adk_telemetry.py b/solutions/ess-maker-skills/scripts/adk_telemetry.py index 7d20c8aa..87b854dd 100644 --- a/solutions/ess-maker-skills/scripts/adk_telemetry.py +++ b/solutions/ess-maker-skills/scripts/adk_telemetry.py @@ -113,6 +113,7 @@ # restore_template_configs-> Workday template-config restore # publishing -> push / deploy to Copilot Studio # flightcheck -> pre-deployment readiness check +# harden -> agent system-instruction hardening review ADK_CAPABILITIES = ( "setup", "connect", @@ -129,6 +130,7 @@ "restore_template_configs", "publishing", "flightcheck", + "harden", ) _CAPABILITY_SET = frozenset(ADK_CAPABILITIES) diff --git a/solutions/ess-maker-skills/scripts/check_instruction_budget.py b/solutions/ess-maker-skills/scripts/check_instruction_budget.py new file mode 100644 index 00000000..3ffc7def --- /dev/null +++ b/solutions/ess-maker-skills/scripts/check_instruction_budget.py @@ -0,0 +1,166 @@ +#!/usr/bin/env python3 +""" +check_instruction_budget.py — Deterministically measure an agent's system +instructions against the character budget. + +Hardening only ever *adds* text. Copilot Studio constrains how long an agent's +instructions may be, so a hardening pass that does not measure will happily +produce instructions that cannot be saved — or that get silently truncated, +which is worse than not hardening at all (a truncated prompt can lose the very +guardrail that was just added). + +Asking the agent to "count the characters" is model-dependent and was observed +to drift. This script removes that variable: it reads the ``instructions`` block +out of ``agent.mcs.yml``, measures it, compares it against the baseline copy so +the maker can see what a pass *added*, and emits a machine-readable verdict the +skill reads verbatim. + +The limit is a **working assumption, not a verified platform constant**. It +defaults to 8000 and is overridable with ``--limit`` so a maker who knows their +real ceiling is not blocked by ours. + +Usage (from solutions/ess-maker-skills/): + python scripts/check_instruction_budget.py --agent employee-self-service-hr + python scripts/check_instruction_budget.py --agent employee-self-service-hr --candidate .local/harden/candidate.txt + python scripts/check_instruction_budget.py --agent employee-self-service-hr --limit 6000 + +Emits a human-readable summary plus a machine-readable block behind a sentinel: + + ###INSTRUCTION_BUDGET_JSON###{"verdict": "ok", "chars": 5996, ...} +""" + +import argparse +import json +import sys +from pathlib import Path + +try: + import yaml +except ImportError: # pragma: no cover - environment guard + yaml = None + +if sys.stdout.encoding and sys.stdout.encoding.lower() != "utf-8": + sys.stdout.reconfigure(encoding="utf-8", errors="replace") + +_SENTINEL = "###INSTRUCTION_BUDGET_JSON###" + +DEFAULT_LIMIT = 8000 + +# Headroom below which a maker should be warned that the next edit will not fit. +# Not a failure — a "you are nearly out of room" signal, so the skill can tell +# the maker to plan a removal before proposing another addition. +TIGHT_HEADROOM = 250 + +_AGENTS_DIR = Path(__file__).resolve().parent.parent / "workspace" / "agents" + + +def _read_instructions(path): + """Return (instructions, error). ``instructions`` is None when unreadable. + + A missing ``instructions:`` key and an empty one are different problems, so + they return different messages — an empty block usually means extraction + ran against an agent that was never configured, which is worth saying out + loud rather than reporting as "0 characters, plenty of headroom". + """ + if not path.is_file(): + return None, f"{path} not found" + if yaml is None: + return None, "PyYAML is not installed in this environment" + try: + data = yaml.safe_load(path.read_text(encoding="utf-8")) + except yaml.YAMLError as exc: + return None, f"{path.name} could not be parsed as YAML: {exc}" + if not isinstance(data, dict): + return None, f"{path.name} did not parse to a mapping" + if "instructions" not in data: + return None, f"{path.name} has no 'instructions' block" + value = data["instructions"] + if value is None: + return None, f"{path.name} has an empty 'instructions' block" + return str(value), None + + +def _verdict(chars, limit): + if chars > limit: + return "over" + if limit - chars < TIGHT_HEADROOM: + return "tight" + return "ok" + + +def main(argv=None): + parser = argparse.ArgumentParser( + description="Measure agent instructions against the character budget." + ) + parser.add_argument("--agent", required=True, help="agent folder under workspace/agents/") + parser.add_argument( + "--candidate", + help="path to a file holding proposed replacement instructions " + "(plain text, not YAML); measured instead of the live block", + ) + parser.add_argument("--limit", type=int, default=DEFAULT_LIMIT, + help=f"character ceiling (default {DEFAULT_LIMIT})") + args = parser.parse_args(argv) + + if args.limit <= 0: + print("--limit must be a positive number of characters", file=sys.stderr) + print(_SENTINEL + json.dumps({"verdict": "unknown", "error": "invalid --limit"})) + return 2 + + agent_dir = _AGENTS_DIR / args.agent + live_path = agent_dir / "agent.mcs.yml" + baseline_path = agent_dir / ".baseline" / "agent.mcs.yml" + + result = { + "agent": args.agent, + "limit": args.limit, + "source": "candidate" if args.candidate else "working", + } + + # Baseline is advisory context, never fatal: a freshly extracted agent has + # one, but an agent mid-edit may not, and that must not block the measure. + baseline_text, _ = _read_instructions(baseline_path) + result["baseline_chars"] = len(baseline_text) if baseline_text is not None else None + + if args.candidate: + cand = Path(args.candidate) + if not cand.is_file(): + print(f"Candidate file not found: {cand}", file=sys.stderr) + print(_SENTINEL + json.dumps({**result, "verdict": "unknown", + "error": "candidate not found"})) + return 2 + text = cand.read_text(encoding="utf-8") + error = None + else: + text, error = _read_instructions(live_path) + + if text is None: + print(f"Could not measure instructions: {error}", file=sys.stderr) + print(_SENTINEL + json.dumps({**result, "verdict": "unknown", "error": error})) + return 2 + + chars = len(text) + headroom = args.limit - chars + result.update({ + "chars": chars, + "headroom": headroom, + "verdict": _verdict(chars, args.limit), + "delta": (chars - result["baseline_chars"]) + if result["baseline_chars"] is not None else None, + }) + + print(f"Instructions: {chars} characters (limit {args.limit}, headroom {headroom})") + if result["delta"] is not None: + sign = "+" if result["delta"] >= 0 else "" + print(f"Change vs. last extract: {sign}{result['delta']} characters") + if result["verdict"] == "over": + print(f"OVER BUDGET by {-headroom} characters — this will not fit.") + elif result["verdict"] == "tight": + print(f"Within budget, but only {headroom} characters remain.") + + print(_SENTINEL + json.dumps(result)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/solutions/ess-maker-skills/src/reference/ess-docs/hardening/instruction-rules.md b/solutions/ess-maker-skills/src/reference/ess-docs/hardening/instruction-rules.md new file mode 100644 index 00000000..ff033064 --- /dev/null +++ b/solutions/ess-maker-skills/src/reference/ess-docs/hardening/instruction-rules.md @@ -0,0 +1,196 @@ +# Instruction hardening rules + +Guidance for reviewing an agent's **system instructions** (the `instructions:` block in +`agent.mcs.yml`) for the patterns that let an agent produce confident, ungrounded, or +over-committing answers — and for the contradictions that make any other rule unenforceable. + +> **Internal vocabulary.** The `INSTR-*` ids below are for keeping findings straight while you +> work. Never show them to the maker. Describe each finding in plain language, quoting the +> maker's own instruction text. + +## How to use this + +Every finding must quote the **exact line** it comes from. A finding you cannot anchor to a +specific line is not a finding — do not report it. Instructions are prose, so the temptation to +report a vague impression ("the tone section feels permissive") is high; resist it. If you cannot +point at the text, you cannot propose a diff for it. + +Two ideas govern the whole rule set: + +1. **A rule that contradicts another rule is not a rule.** The model resolves the conflict, not + the author. Contradictions are the highest-value finding class because they silently disable + guardrails the author believes are in force. +2. **Govern what a sentence does, not how it is worded.** Phrase blacklists fail. An agent told + never to say "Would you like" will say "I'd be happy to." Rules must name the *function* being + prohibited — offering, asserting, referring — and say that rewording does not exempt it. + +--- + +## Part 1 — Contradictions (always check) + +Run this pass regardless of what the maker reports. It is the one pass that is valuable even when +the agent is behaving well. + +### INSTR-001 — Direct contradiction + +Two instructions that cannot both be satisfied. Classic shape: a broad prohibition elsewhere +contradicted by a specific permission, e.g. a rule forbidding referral to *any* outside +organization alongside a support section that explicitly endorses naming an employee assistance +program. + +**Report both sides.** A contradiction has no single guilty line, and proposing a fix to only one +half usually breaks the behavior the other half was protecting. + +### INSTR-002 — Scope collision + +A general rule whose plain reading swallows a legitimate case the agent must still handle. +Frequently produced by bundling unrelated prohibitions into one sentence: prohibiting the agent +from *creating* an exception and from *explaining a documented exception process* in the same +breath, when only the first is unwanted. + +**Fix by splitting**, not by narrowing into ambiguity. + +### INSTR-003 — Precedence gap + +Two rules conflict and nothing says which wins. Any instruction meant to be absolute must say so +and say what it outranks. "Be helpful and warm" and "never state a fact you did not retrieve" +will collide constantly; without precedence, tone usually wins, because tone rules are stated +first and phrased more confidently. + +### INSTR-004 — Unreachable or vacuous rule + +A rule that cannot be evaluated at answer time — "format the response appropriately wherever +required", "use good judgment". These consume budget and teach the model that instructions are +suggestions. Removing them is nearly always safe and frees characters for rules that bind. + +### INSTR-005 — Duplicate rule stated with different strength + +The same requirement appears twice with different force ("cite sources" and "always cite the +specific source"). The weaker statement gives the model a defensible reading of the stronger one. +Consolidate to one statement at the intended strength. + +--- + +## Part 2 — Ungrounded-response risk + +### INSTR-010 — Unconditional confidence + +Tone guidance instructing the agent to be confident, authoritative, or direct **without +conditioning it on having retrieved something**. The agent is most confident exactly when it +should hedge — when retrieval returned nothing. Make confidence conditional. + +### INSTR-011 — Sanctioned ungrounded answer mode + +Any wording that legitimizes answering from something other than the knowledge sources: "general +guidance", "background information", "general knowledge where appropriate". This creates a +sanctioned lane for fabrication and is often *adjacent* to a strong grounding rule, which is what +makes it dangerous — the author reads the strong rule, the model uses the lane. + +### INSTR-012 — Pattern inference across sources + +Nothing forbids assembling a plausible detail from patterns seen across *other* retrieved +content. This output looks grounded — every ingredient came from the corpus — so grounding rules +phrased as "do not use outside knowledge" do not catch it. Require lookup per detail, and state +that a detail which was not retrieved is unknown regardless of how predictable it seems. + +### INSTR-013 — Unbounded jurisdiction or population scope + +The agent may answer region-, country-, or population-specific questions with sources that do not +cover that scope. Especially likely where instructions *encourage* reasoning about regional +variation without also constraining the source of that reasoning. + +### INSTR-014 — Legal, regulatory, or entitlement assertions + +No rule prevents stating what a law requires or what someone is entitled to. High-consequence for +HR, benefits, payroll, and leave agents. + +### INSTR-015 — Referral to external authorities + +No rule prevents directing users to a government agency, regulator, or court. Distinguish +carefully from **endorsed** support resources named in approved content, which must stay allowed — +see INSTR-001. + +--- + +## Part 3 — Over-committing and capability overreach + +### INSTR-020 — Unsolicited offers + +Nothing stops the agent from ending a response by offering an action, service, or calculation it +was not asked for. An offer commits the organization to something no source authorized, which +makes an unsolicited offer a factual error even when the wording is friendly and even when the +answer preceding it was correct. + +Check whether an existing rule tries to control this **by phrase list**. If it does, that is +itself the finding — the list is evidence the problem was recognized and the fix did not hold. + +### INSTR-021 — Capability inference + +Nothing constrains the agent to actions its configured tools and topics actually support, so it +infers capability from what systems of that kind usually do. Because a maker's tool inventory +changes, prefer an abstract rule ("unless a configured tool supports it") over enumerating +specific actions the agent cannot perform — an enumeration goes stale and, worse, can prohibit +something the agent genuinely does support. + +### INSTR-022 — Standing helpfulness pressure + +Instructions establishing that the agent is always "ready to help", should "look for ways to +assist", or similar. This reads as tone but behaves as a standing instruction to generate +follow-up offers, and it directly undercuts any rule added for INSTR-020. + +--- + +## Part 4 — Over-restriction (the symmetric failure) + +An agent that refuses valid questions is also a failure, and it is the failure a hardening pass is +most likely to *introduce*. Check proposed changes against these before presenting them. + +### INSTR-030 — Refusal without a grounded alternative + +A prohibition with no statement of what the agent should do instead. Every prohibition needs a +defined fallback — say the sources do not cover it, and escalate by the agent's configured path. + +### INSTR-031 — Missing anti-over-refusal guard + +A strong prohibition block with nothing stating that it restricts *invention*, not helpfulness. +Without that guard the model generalizes the prohibitions outward and begins declining questions +its sources fully answer. An unnecessary refusal is as much a failure as an unsupported answer, +and instructions should say so explicitly. + +### INSTR-032 — Prohibition that blocks a supported action + +A proposed rule that would prevent the agent from doing something its configured topics and tools +support. **This is the most damaging change a hardening pass can make**, because it is invisible +in review — the instructions read as responsible — and only shows up as users being turned away. +When the tool and topic inventory is unknown, keep capability rules abstract rather than guessing +at what the agent cannot do. + +### INSTR-033 — Conversational collapse + +Prohibitions on endings that also remove necessary clarifying questions or required escalation +instructions. Carve those out explicitly; they are not offers. + +--- + +## Part 5 — Rewriting principles + +When proposing a change: + +- **Quote the line you are replacing.** Show old and new, never a summary of the change. +- **Prefer removal to addition.** The budget is finite and a removed permissive line is often + worth more than an added prohibition. Removing "always ready to help" does more for INSTR-020 + than any new sentence. +- **State the reason in the instruction itself** when it is cheap. A rule carrying its rationale + survives paraphrase by a future editor; a bare imperative gets "cleaned up" and lost. +- **Put absolute rules early and say what they outrank.** Order matters, and later text does not + reliably override earlier text. +- **Never add a phrase blacklist as the primary mechanism.** It may support a functional rule; it + cannot replace one. +- **Do not invent an escalation mechanism.** Use the agent's existing configured path. Naming a + channel, link, or process the maker never configured is itself an ungrounded instruction. + +## Part 6 — Product-default findings + +Instructions derived from a shipped ESS template commonly carry INSTR-010, INSTR-011, INSTR-013, +and INSTR-022 unmodified. Finding them is expected and is not evidence the maker did anything +wrong — say so when reporting, or the report reads as an accusation about text they never wrote. diff --git a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md new file mode 100644 index 00000000..05bcc1da --- /dev/null +++ b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md @@ -0,0 +1,226 @@ +# Harden Instructions Skill + +This skill reviews an agent's **system instructions** (the `instructions:` block in `agent.mcs.yml`) and +proposes changes that reduce two failure modes: + +- **Ungrounded answers** — the agent states something its knowledge sources do not support. +- **Over-committing answers** — the agent offers an action, service, or referral that nothing authorized. + +It also always checks the instructions for **internal contradictions**, which are worth fixing even when +the agent is behaving well: a rule contradicted elsewhere is not in force, however firmly it is written. + +> **Advisory and diff-based.** Every change is shown as exact before-and-after text and applied only after +> the maker approves. Instructions govern every answer the agent gives, so an unreviewed edit here is far +> more dangerous than an unreviewed edit to a single topic. + +## Rules + +- **Never rewrite the instructions wholesale.** Propose the smallest set of line-level changes that address + what was actually found. A rewrite is unreviewable — the maker cannot tell an intended change from an + incidental one — and it discards wording their organization may have chosen deliberately. +- **Do not tighten just because you were invoked.** If the review finds nothing and the maker reports no + problem, say so and stop. Adding prohibitions "to be safe" causes the agent to refuse questions its + sources fully answer, which is a real regression traded for a hypothetical one. +- **Quote before proposing.** Every finding and every proposed change names the exact line it applies to. + Findings you cannot anchor to a line do not get reported. +- **Never propose a rule that blocks a supported action.** Check the agent's configured topics and tools + before prohibiting anything the agent might legitimately do (see `INSTR-032`). +- **Never apply changes without a checkpoint** (Step 7). +- **Do not push.** This skill writes locally. Pushing is the maker's separate, explicit decision via `/push`. +- **Run the analysis silently.** Steps 3–5 are internal. Do not narrate which files you are reading, which + rule ids matched, or what you are about to check. The maker sees Step 6 onward. +- **Speak the maker's language.** Never show `INSTR-*` ids, rule-pack filenames, or the words "detector", + "rule pack", or "probe". Describe each finding in plain language and quote the maker's own text. Their + instruction wording is *their* language and should be shown in full. +- **TRACK PROGRESS**: use the todo list tool to track the steps below so the maker can see where you are. + +## What this checks and what it does not + +This skill reads **only the instructions text**, plus the agent's topic and tool inventory for the +capability checks. It **cannot** tell you whether the agent actually produces a bad answer — instructions +are one input to that, and retrieval quality, knowledge-source coverage, and the underlying model matter at +least as much. + +Say this plainly when it is relevant. A maker whose agent gives ungrounded answers because a knowledge +source is not being retrieved will get no benefit from tighter instructions, and letting them believe +otherwise costs them the time they should have spent on retrieval. Two signals that instructions are the +wrong lever: + +- the agent answers correctly when the maker pastes the source content into the chat, but not otherwise; +- the agent says it cannot find information that the maker knows is in an attached source. + +Both point at knowledge-source configuration. Route those makers to `/flightcheck` and `/troubleshoot` +rather than editing instructions. + +## Step 1: Resolve the agent + +Read `.local/config.json` for the agent folder. The instructions live at +`workspace/agents/{agent.folder}/agent.mcs.yml` in the `instructions:` block. + +If the file is missing, tell the maker their agent has not been extracted yet and to run `/setup`, then STOP. + +If the `instructions:` block is missing or empty, say so and STOP — there is nothing to harden, and this +usually means the agent's instructions were never configured rather than that they are safe. + +## Step 2: Ask what the maker has actually seen + +Ask before analyzing. What the maker has observed is better evidence than anything derivable from the text, +and it determines whether Step 6 proposes changes or only reports. + +> Before I look at your instructions — have you seen specific answers from your agent that you didn't like? +> +> If you can paste one or two, that helps most: the exact question and what the agent said. Otherwise, tell +> me the kind of answer you want to prevent — for example, making claims your documents don't cover, or +> offering to do things the agent can't actually do. +> +> If nothing specific has gone wrong, that's fine too — say so and I'll check the instructions for +> contradictions and gaps and tell you what I find. + +Record their answer. Do not paraphrase a vague answer into a specific complaint — if they said "it makes +things up sometimes" without an example, you have a **theme**, not a case, and Step 6 treats those +differently. + +## Step 3: Read the instructions and the rule pack + +Read the full `instructions:` value and the rule pack at +[`instruction-rules.md`](src/reference/ess-docs/hardening/instruction-rules.md). + +Split the instructions into numbered lines so every finding can be anchored precisely. Keep the maker's +original wording, spelling, and casing exactly — you will be quoting it back and later diffing against it. + +## Step 4: Contradiction pass (always runs) + +Apply Part 1 of the rule pack (`INSTR-001` … `INSTR-005`). This pass runs regardless of the maker's answer +in Step 2. + +For each contradiction, record **both** conflicting lines. Do not decide which one is "right" — the maker +knows which behavior they intended, and guessing produces a fix that removes a guardrail they wanted. + +## Step 5: Grounding and over-commitment pass + +Apply Parts 2 and 3 of the rule pack (`INSTR-010` … `INSTR-022`). + +Where the maker gave specific bad responses in Step 2, work backward from each one: identify which +instruction *permitted* it, or which instruction that would have prevented it is **absent**. An absence is a +valid finding as long as you can state the specific behavior that is unconstrained. + +For capability findings (`INSTR-021`), read the agent's topic files (`{agent.folder}/topics/`) and tool +inventory first. You need to know what the agent *can* do before writing a rule about what it cannot. + +For any finding where an existing rule already targets the reported behavior **by listing forbidden +phrases**, record that the mechanism itself failed. Phrase lists are evaded by rewording; the replacement +must prohibit the *function* — offering, asserting, referring — and say that rephrasing does not exempt it. + +## Step 6: Decide what to propose + +Branch on Step 2: + +**A — the maker described specific responses or a specific behavior.** +Propose targeted changes for those, plus any contradictions from Step 4. Every proposed change must trace to +either a reported behavior or a contradiction. Do not append unrelated hardening because the file happened +to be open. + +**B — the maker reported nothing specific.** +Propose fixes for **contradictions** (Step 4) and for findings where a rule is **internally inconsistent or +vacuous** (`INSTR-004`, `INSTR-005`). Report the grounding and over-commitment findings from Step 5 as +observations with the risk each carries, and ask whether the maker wants any of them addressed. Do not +propose those changes pre-approved. + +The reason is worth stating to the maker if they push back: prohibitions have a cost. Each one makes the +agent more likely to decline a question it could have answered, and without a reported problem there is +nothing to weigh that cost against. + +Before finalizing any proposal, check it against Part 4 of the rule pack (`INSTR-030` … `INSTR-033`). Any +prohibition needs a stated alternative behavior, and the proposal as a whole needs a line stating the +prohibitions restrict invention rather than helpfulness. + +## Step 7: Check the character budget + +Instructions have a length ceiling in Copilot Studio, and hardening only adds text. Write the proposed full +instructions to `.local/harden/candidate.txt`, then run: + +``` +python scripts/check_instruction_budget.py --agent {agent.folder} --candidate .local/harden/candidate.txt +``` + +Read the `###INSTRUCTION_BUDGET_JSON###` line. **Its verdict is authoritative — do not estimate the length +yourself and do not override it.** + +- `ok` — proceed. +- `tight` — proceed, and tell the maker how little room is left. +- `over` — **do not present the proposal as-is.** Identify what to remove and propose that too. Prefer + removing permissive or vacuous lines (`INSTR-004`, `INSTR-011`, `INSTR-022`) — removing a line that invites + the bad behavior is usually worth more than the prohibition you were trying to add. Re-run until `ok`. + +The default limit is the kit's working assumption, not a verified platform constant. If the maker knows +their real ceiling, pass it with `--limit`. + +## Step 8: Present the proposal and get approval + +Present, in this order: + +1. **What you found**, in plain language, grouped as contradictions first, then risks. Quote the maker's + own line for each. If the instructions came from a shipped template, say that where it applies — several + common findings are inherited defaults, not something the maker wrote. +2. **What you propose to change**, as before-and-after pairs: + + > **Currently:** "{exact original line}" + > **Proposed:** "{exact replacement}" + > **Why:** {one or two sentences tied to what this prevents} + + For a removal, show the line and say it is being removed and what that changes. +3. **The length**, in one line: the new total and the remaining headroom. +4. **What this does not cover**: instructions do not fix a knowledge source the agent cannot retrieve. If + the maker's reported examples looked like retrieval problems (see "What this checks"), say so here. + +Then ask for approval. The maker may accept all, accept some, or decline. Apply exactly what they accept. + +If there is nothing to propose, say so directly and stop — do not manufacture a finding to justify the run. + +## Step 9: Apply + +Only after explicit approval: + +``` +python scripts/checkpoint.py "pre-harden-instructions" +python scripts/emit_capability.py harden +``` + +The `emit_capability.py` line records anonymous usage telemetry (best-effort, non-blocking); it needs no +user-facing message and never fails the step. + +Then edit the `instructions:` block in `workspace/agents/{agent.folder}/agent.mcs.yml`, changing only the +approved lines. Preserve the YAML block scalar style and the indentation of the surrounding file. + +Preserve **exactly** any `{System.Bot.Components.Topics...}` references or other placeholder tokens in the +instructions. These are live references, not example text — rewording one silently breaks the behavior it +drives. + +Re-run the budget check without `--candidate` to confirm the written file measures as expected. + +Delete `.local/harden/candidate.txt` once applied. + +## Step 10: Hand off to validation + +Instruction changes are behavioral changes, and this skill has no way to demonstrate that the new text +produces better answers. Say that plainly and route the maker onward: + +> These changes aren't verified yet — instructions affect every answer, so it's worth checking the agent +> still behaves the way you want. +> +> - `/evaluate` — turn the answers you didn't like into test cases, so you can tell whether this fixed them +> - `/test` — try the agent's behaviour directly +> - `/push` — send the change to Copilot Studio when you're ready + +Where the maker gave specific bad responses in Step 2, carry them forward: those are the highest-value +evaluation rows available, and they are the only direct evidence of whether this pass worked. Offer to run +`/evaluate` with them. + +Also mention, once, that a change intended to prevent a bad answer can also cause the agent to decline good +questions — and that a few normal, in-scope questions are worth testing alongside the failing ones. + +## References + +- [`instruction-rules.md`](src/reference/ess-docs/hardening/instruction-rules.md) — the rule pack: + contradiction classes, grounding and over-commitment risks, over-restriction risks, and rewriting + principles. diff --git a/tests/scripts/test_instruction_budget.py b/tests/scripts/test_instruction_budget.py new file mode 100644 index 00000000..5695fd58 --- /dev/null +++ b/tests/scripts/test_instruction_budget.py @@ -0,0 +1,243 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +""" +Unit tests for ``check_instruction_budget`` — the deterministic probe the +``/harden`` skill relies on to decide whether proposed instructions fit. + +Why this is probed rather than reasoned about: hardening only ever *adds* +text, and instructions that exceed the ceiling can be silently truncated, +which can drop the guardrail the pass just added. The skill is told the +probe's verdict outranks its own estimate, so these tests lock down the +contract that makes that safe — a sentinel line is always emitted, and the +error paths report ``unknown`` rather than a falsely clean measurement. +""" + +from __future__ import annotations + +import json +import shutil +import subprocess +import sys +import uuid +from pathlib import Path + +import pytest + +import check_instruction_budget as budget + +_SKILL_ROOT = Path(budget.__file__).resolve().parent.parent +_SCRIPT = _SKILL_ROOT / "scripts" / "check_instruction_budget.py" +_AGENTS_DIR = _SKILL_ROOT / "workspace" / "agents" + +_SENTINEL = "###INSTRUCTION_BUDGET_JSON###" + + +def _agent_yaml(instructions: str) -> str: + body = "\n".join(" " + line for line in instructions.splitlines()) + return f"kind: GptComponentMetadata\ninstructions: |-\n{body}\n" + + +@pytest.fixture +def temp_agent(): + """A throwaway agent with a working and a baseline copy of its instructions. + + Lives under the gitignored workspace/agents/ tree so it leaves no git + trace; removed on teardown. + """ + name = f"_pytest_{uuid.uuid4().hex}" + agent_dir = _AGENTS_DIR / name + (agent_dir / ".baseline").mkdir(parents=True) + + (agent_dir / "agent.mcs.yml").write_text( + _agent_yaml("You are an HR agent.\nAnswer only from your sources."), + encoding="utf-8", + ) + (agent_dir / ".baseline" / "agent.mcs.yml").write_text( + _agent_yaml("You are an HR agent."), + encoding="utf-8", + ) + try: + yield name + finally: + shutil.rmtree(agent_dir, ignore_errors=True) + + +def _run(*args) -> tuple[int, dict, str]: + proc = subprocess.run( + [sys.executable, str(_SCRIPT), *args], + capture_output=True, + text=True, + encoding="utf-8", + ) + combined = proc.stdout + proc.stderr + lines = [ln for ln in proc.stdout.splitlines() if ln.startswith(_SENTINEL)] + # Every path must emit exactly one verdict; the skill parses this line and + # has no fallback if it is missing or duplicated. + assert len(lines) == 1, f"expected one sentinel line, got {len(lines)}: {combined}" + return proc.returncode, json.loads(lines[0][len(_SENTINEL):]), combined + + +# --------------------------------------------------------------------------- # +# verdict thresholds +# --------------------------------------------------------------------------- # + +@pytest.mark.parametrize( + "chars, limit, expected", + [ + (100, 8000, "ok"), + (7999, 8000, "tight"), # under the limit but inside the warning band + (8000, 8000, "tight"), # exactly at the limit is not over + (8001, 8000, "over"), + (7751, 8000, "tight"), # 249 left -> still inside the band + (7750, 8000, "ok"), # 250 left -> the first value outside it + ], +) +def test_verdict_thresholds(chars, limit, expected): + assert budget._verdict(chars, limit) == expected + + +def test_limit_boundary_is_inclusive(): + """Exactly at the limit is not over — an off-by-one here would send the + skill hunting for text to remove from instructions that already fit.""" + assert budget._verdict(8000, 8000) != "over" + + +# --------------------------------------------------------------------------- # +# measuring a real agent +# --------------------------------------------------------------------------- # + +def test_measures_working_instructions(temp_agent): + code, result, combined = _run("--agent", temp_agent) + assert code == 0, combined + assert result["verdict"] == "ok" + assert result["source"] == "working" + assert result["chars"] == len("You are an HR agent.\nAnswer only from your sources.") + assert result["headroom"] == result["limit"] - result["chars"] + + +def test_reports_delta_against_baseline(temp_agent): + """The maker needs to see what a pass *added*, not just the total.""" + _, result, _ = _run("--agent", temp_agent) + assert result["baseline_chars"] == len("You are an HR agent.") + assert result["delta"] == result["chars"] - result["baseline_chars"] + + +def test_candidate_file_is_measured_instead(temp_agent, tmp_path): + candidate = tmp_path / "candidate.txt" + candidate.write_text("x" * 12, encoding="utf-8") + + _, result, _ = _run("--agent", temp_agent, "--candidate", str(candidate)) + assert result["source"] == "candidate" + assert result["chars"] == 12 + + +def test_over_budget_is_reported(temp_agent, tmp_path): + candidate = tmp_path / "candidate.txt" + candidate.write_text("x" * 50, encoding="utf-8") + + code, result, combined = _run( + "--agent", temp_agent, "--candidate", str(candidate), "--limit", "40" + ) + assert code == 0, combined + assert result["verdict"] == "over" + assert result["headroom"] == -10 + assert "OVER BUDGET" in combined + + +def test_custom_limit_is_honoured(temp_agent): + """The ceiling is the kit's working assumption, so a maker who knows their + real one must be able to override it rather than be blocked by ours.""" + _, result, _ = _run("--agent", temp_agent, "--limit", "6000") + assert result["limit"] == 6000 + + +# --------------------------------------------------------------------------- # +# error paths never produce a falsely clean verdict +# --------------------------------------------------------------------------- # + +def test_missing_agent_reports_unknown(): + code, result, _ = _run("--agent", "_pytest_does_not_exist") + assert code != 0 + assert result["verdict"] == "unknown" + assert "chars" not in result + + +def test_missing_instructions_block_reports_unknown(temp_agent): + (_AGENTS_DIR / temp_agent / "agent.mcs.yml").write_text( + "kind: GptComponentMetadata\n", encoding="utf-8" + ) + code, result, _ = _run("--agent", temp_agent) + assert code != 0 + assert result["verdict"] == "unknown" + + +def test_empty_instructions_block_reports_unknown(temp_agent): + """An empty block means the agent was never configured. Reporting it as + "0 characters, plenty of headroom" would read as a clean pass.""" + (_AGENTS_DIR / temp_agent / "agent.mcs.yml").write_text( + "kind: GptComponentMetadata\ninstructions:\n", encoding="utf-8" + ) + code, result, _ = _run("--agent", temp_agent) + assert code != 0 + assert result["verdict"] == "unknown" + + +def test_unparseable_yaml_reports_unknown(temp_agent): + (_AGENTS_DIR / temp_agent / "agent.mcs.yml").write_text( + "instructions: [unclosed\n", encoding="utf-8" + ) + code, result, _ = _run("--agent", temp_agent) + assert code != 0 + assert result["verdict"] == "unknown" + + +def test_missing_candidate_reports_unknown(temp_agent, tmp_path): + code, result, _ = _run( + "--agent", temp_agent, "--candidate", str(tmp_path / "nope.txt") + ) + assert code != 0 + assert result["verdict"] == "unknown" + + +def test_missing_baseline_still_measures(temp_agent): + """A baseline is advisory context. Its absence must not block the measure — + an agent mid-edit may not have one.""" + (_AGENTS_DIR / temp_agent / ".baseline" / "agent.mcs.yml").unlink() + + code, result, combined = _run("--agent", temp_agent) + assert code == 0, combined + assert result["verdict"] == "ok" + assert result["baseline_chars"] is None + assert result["delta"] is None + + +def test_invalid_limit_is_rejected(temp_agent): + code, result, _ = _run("--agent", temp_agent, "--limit", "0") + assert code != 0 + assert result["verdict"] == "unknown" + + +# --------------------------------------------------------------------------- # +# wiring +# --------------------------------------------------------------------------- # + +def test_harden_prompt_and_skill_are_present(): + """The prompt delegates to the skill by path; a rename that breaks the + link is silent at runtime — the maker just gets an unguided response.""" + prompt = _SKILL_ROOT / ".github" / "prompts" / "harden.prompt.md" + skill = _SKILL_ROOT / "src" / "skills" / "instructions" / "harden" / "SKILL.md" + rules = (_SKILL_ROOT / "src" / "reference" / "ess-docs" / "hardening" + / "instruction-rules.md") + + assert prompt.is_file() + assert skill.is_file() + assert rules.is_file() + + prompt_text = prompt.read_text(encoding="utf-8") + assert "src/skills/instructions/harden/SKILL.md" in prompt_text + + skill_text = skill.read_text(encoding="utf-8") + assert "src/reference/ess-docs/hardening/instruction-rules.md" in skill_text + assert "scripts/check_instruction_budget.py" in skill_text + assert "emit_capability.py harden" in skill_text diff --git a/tests/test_adk_telemetry.py b/tests/test_adk_telemetry.py index f004c33f..e8830efc 100644 --- a/tests/test_adk_telemetry.py +++ b/tests/test_adk_telemetry.py @@ -648,6 +648,7 @@ def test_wired_capabilities_are_in_canonical_list(): "topic_create", "topic_update", "topic_delete", "workflow_create", "workflow_update", "workflow_delete", "cleanup", "troubleshoot", + "harden", } missing = wired - set(adk.ADK_CAPABILITIES) assert not missing, f"wired capabilities not in ADK_CAPABILITIES: {missing}" From 95e50be30528b8b0c26c8d81ee0ebaefbeabd153 Mon Sep 17 00:00:00 2001 From: Avery Milandin Date: Mon, 24 Aug 2026 15:35:38 -0700 Subject: [PATCH 02/10] fix(harden): address defects found in end-to-end validation Two simulated end-to-end runs of /harden against a real extracted agent surfaced defects that reading the files could not. - list_agent_capabilities.py: a new probe that reports the agent's topics, workflows and acting steps. Capability checks previously had no resolvable source and were guesswork. Uses a line-based key extractor because Copilot Studio topic YAML contains unquoted '@type' keys and is not valid YAML -- yaml.safe_load fails on exactly the integration topics that matter most. - Keep descriptions out of the probe's JSON. Duplicating them into both the table and the JSON pushed a real agent's output past the caller's limit. - check_instruction_budget.py: accept a slug, a relative path or an absolute path for --agent. config.json stores agent.folder as a path, so the skill's documented invocation produced a doubled path. - SKILL.md: anchor findings at sentence level, state that a missing safeguard is a valid finding without a quoted line, require a concrete failing request before reporting a contradiction, and bound the capability review. - instruction-rules.md: correct the shipped-template claims against the instructions the template actually ships, and stop INSTR-003 from contradicting the contradiction threshold. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 96774230-f664-4887-baf1-a5bbfbd6cbb2 --- .../scripts/check_instruction_budget.py | 35 ++- .../scripts/list_agent_capabilities.py | 210 ++++++++++++++++++ .../ess-docs/hardening/instruction-rules.md | 84 +++++-- .../src/skills/instructions/harden/SKILL.md | 203 +++++++++++------ tests/scripts/test_agent_capabilities.py | 196 ++++++++++++++++ tests/scripts/test_instruction_budget.py | 17 ++ 6 files changed, 653 insertions(+), 92 deletions(-) create mode 100644 solutions/ess-maker-skills/scripts/list_agent_capabilities.py create mode 100644 tests/scripts/test_agent_capabilities.py diff --git a/solutions/ess-maker-skills/scripts/check_instruction_budget.py b/solutions/ess-maker-skills/scripts/check_instruction_budget.py index 3ffc7def..ff43c548 100644 --- a/solutions/ess-maker-skills/scripts/check_instruction_budget.py +++ b/solutions/ess-maker-skills/scripts/check_instruction_budget.py @@ -3,11 +3,11 @@ check_instruction_budget.py — Deterministically measure an agent's system instructions against the character budget. -Hardening only ever *adds* text. Copilot Studio constrains how long an agent's -instructions may be, so a hardening pass that does not measure will happily -produce instructions that cannot be saved — or that get silently truncated, -which is worse than not hardening at all (a truncated prompt can lose the very -guardrail that was just added). +Hardening usually lengthens instructions. Copilot Studio constrains how long an +agent's instructions may be, so a hardening pass that does not measure will +happily produce instructions that cannot be saved — or that get silently +truncated, which is worse than not hardening at all (a truncated prompt can +lose the very guardrail that was just added). Asking the agent to "count the characters" is model-dependent and was observed to drift. This script removes that variable: it reads the ``instructions`` block @@ -52,6 +52,25 @@ TIGHT_HEADROOM = 250 _AGENTS_DIR = Path(__file__).resolve().parent.parent / "workspace" / "agents" +_SKILL_ROOT = Path(__file__).resolve().parent.parent + + +def _resolve_agent_dir(value): + """Resolve --agent from any of the forms a caller reasonably has to hand. + + ``.local/config.json`` stores ``agent.folder`` as a path relative to the + solution root (``workspace/agents/``) and ``activeAgent`` as a bare + slug. Accepting only one of them means whichever the caller reaches for + first is a coin flip, and the failure is an unhelpful "not found" against a + doubled-up path. + """ + candidate = Path(value) + if candidate.is_absolute(): + return candidate + relative_to_root = _SKILL_ROOT / value + if relative_to_root.is_dir(): + return relative_to_root + return _AGENTS_DIR / value def _read_instructions(path): @@ -92,7 +111,9 @@ def main(argv=None): parser = argparse.ArgumentParser( description="Measure agent instructions against the character budget." ) - parser.add_argument("--agent", required=True, help="agent folder under workspace/agents/") + parser.add_argument("--agent", required=True, + help="agent folder name under workspace/agents/, or the " + "'agent.folder' path from .local/config.json") parser.add_argument( "--candidate", help="path to a file holding proposed replacement instructions " @@ -107,7 +128,7 @@ def main(argv=None): print(_SENTINEL + json.dumps({"verdict": "unknown", "error": "invalid --limit"})) return 2 - agent_dir = _AGENTS_DIR / args.agent + agent_dir = _resolve_agent_dir(args.agent) live_path = agent_dir / "agent.mcs.yml" baseline_path = agent_dir / ".baseline" / "agent.mcs.yml" diff --git a/solutions/ess-maker-skills/scripts/list_agent_capabilities.py b/solutions/ess-maker-skills/scripts/list_agent_capabilities.py new file mode 100644 index 00000000..8f67827d --- /dev/null +++ b/solutions/ess-maker-skills/scripts/list_agent_capabilities.py @@ -0,0 +1,210 @@ +#!/usr/bin/env python3 +""" +list_agent_capabilities.py — Summarize what an agent can actually *do*. + +The riskiest change an instruction-hardening pass can make is prohibiting +something the agent genuinely supports: the instructions read as more +responsible, and the regression only shows up later as users being turned away. +Avoiding that requires knowing the agent's real capabilities. + +Reading the topic files by hand does not scale — a stock ESS agent ships ~36 of +them, several over 40 KB of generated flow detail — so in practice the check +gets skipped or done from a sample, and two runs reach different conclusions. +This script reduces the tree to one screen: per topic, what the model is told it +handles (``modelDescription``), the trigger phrases if any, and whether it +merely replies or invokes a flow, connector, or HTTP call. + +Usage (from solutions/ess-maker-skills/): + python scripts/list_agent_capabilities.py --agent employee-self-service-preview + +Emits a human-readable table plus a machine-readable block behind a sentinel: + + ###AGENT_CAPABILITIES_JSON###{"topics": [...], "workflows": [...]} +""" + +import argparse +import json +import re +import sys +from pathlib import Path + +if sys.stdout.encoding and sys.stdout.encoding.lower() != "utf-8": + sys.stdout.reconfigure(encoding="utf-8", errors="replace") + +_SENTINEL = "###AGENT_CAPABILITIES_JSON###" + +_SKILL_ROOT = Path(__file__).resolve().parent.parent +_AGENTS_DIR = _SKILL_ROOT / "workspace" / "agents" + +# Action kinds that mean the topic reaches a real system rather than just +# replying. The distinction is the whole point: "can answer about X" and "can +# do X" need different instruction rules. +_ACTING_KINDS = { + "InvokeFlowAction": "flow", + "InvokeConnectorAction": "connector", + "HttpRequestAction": "http", + "InvokeAIBuilderModelAction": "ai-builder", + "SearchAndSummarizeContent": "knowledge-search", + "BeginDialog": "calls-another-topic", +} +_MAX_DESCRIPTION = 320 + +_KIND_RE = re.compile(r"^\s*(?:-\s*)?kind:\s*([A-Za-z0-9_.]+)\s*$", re.MULTILINE) + + +def _resolve_agent_dir(value): + """Accept a bare slug, a solution-root-relative path, or an absolute path. + + ``.local/config.json`` holds ``agent.folder`` as ``workspace/agents/`` + and ``activeAgent`` as a bare slug; both are natural things for a caller to + pass, so both must work. + """ + candidate = Path(value) + if candidate.is_absolute(): + return candidate + relative_to_root = _SKILL_ROOT / value + if relative_to_root.is_dir(): + return relative_to_root + return _AGENTS_DIR / value + + +def _top_level_value(text, key): + """Read a top-level key's value without parsing the file as YAML. + + Copilot Studio topic files are not valid YAML — they contain unquoted + ``@type:`` keys that a compliant parser rejects — so ``yaml.safe_load`` + fails on exactly the Workday and ServiceNow topics whose descriptions + matter most. The fields needed here are all at column 0, so a line scan is + both more robust and sufficient. + + Handles an inline value and a block scalar (``|``, ``|-``, ``>``). + """ + lines = text.splitlines() + for index, line in enumerate(lines): + if not line.startswith(key + ":"): + continue + inline = line[len(key) + 1:].strip() + if inline and not inline.startswith(("|", ">")): + return inline.strip("'\"") + collected = [] + for following in lines[index + 1:]: + if following.strip() and not following.startswith((" ", "\t")): + break + collected.append(following.strip()) + return " ".join(part for part in collected if part) + return None + + +def _top_level_list(text, key): + """Read a top-level block-sequence value (``key:`` then `` - item`` lines).""" + lines = text.splitlines() + for index, line in enumerate(lines): + if line.rstrip() != key + ":": + continue + items = [] + for following in lines[index + 1:]: + if not following.strip(): + continue + if not following.startswith((" ", "\t")): + break + stripped = following.strip() + if stripped.startswith("- "): + items.append(stripped[2:].strip().strip("'\"")) + else: + break + return items + return [] + + +def _summarize_topic(path): + """Return a capability summary for one topic file. + + Unreadable files are reported, never skipped silently — a topic that could + not be read is a gap in the capability picture, and the caller is about to + write prohibitions based on that picture. + """ + entry = {"file": path.name, "description": None, "triggers": [], "actions": []} + try: + text = path.read_text(encoding="utf-8") + except (UnicodeDecodeError, OSError) as exc: + entry["error"] = f"could not read: {exc.__class__.__name__}" + return entry + + # Action kinds come from a text scan rather than the parse tree: they are + # nested arbitrarily deep and a malformed topic should still disclose what + # it reaches. + kinds = {m.group(1) for m in _KIND_RE.finditer(text)} + entry["actions"] = sorted({_ACTING_KINDS[k] for k in kinds if k in _ACTING_KINDS}) + + description = _top_level_value(text, "modelDescription") or _top_level_value(text, "description") + if description: + collapsed = " ".join(description.split()) + # Descriptions run to a thousand characters of worked examples. The + # opening sentences carry the capability; the rest floods the caller's + # context for no gain. + if len(collapsed) > _MAX_DESCRIPTION: + collapsed = collapsed[:_MAX_DESCRIPTION].rstrip() + " ..." + entry["description_truncated"] = True + entry["description"] = collapsed + entry["triggers"] = _top_level_list(text, "triggerQueries") + return entry + + +def main(argv=None): + parser = argparse.ArgumentParser( + description="Summarize an agent's configured topics and workflows." + ) + parser.add_argument("--agent", required=True, + help="agent folder name under workspace/agents/, or the " + "'agent.folder' path from .local/config.json") + args = parser.parse_args(argv) + + agent_dir = _resolve_agent_dir(args.agent) + if not agent_dir.is_dir(): + print(f"Agent folder not found: {agent_dir}", file=sys.stderr) + print(_SENTINEL + json.dumps({"error": "agent folder not found"})) + return 2 + + topics_dir = agent_dir / "topics" + topics = [_summarize_topic(p) for p in sorted(topics_dir.glob("*.mcs.yml"))] + + workflows_dir = agent_dir / "workflows" + workflows = sorted(p.name for p in workflows_dir.glob("*")) if workflows_dir.is_dir() else [] + + result = { + "agent": args.agent, + "topic_count": len(topics), + "workflow_count": len(workflows), + # Descriptions and triggers stay out of the JSON: they are long, they + # are already in the table above, and duplicating them pushed a real + # agent's output past the caller's output limit — which cost the + # capability evidence this probe exists to provide. + "topics": [{"file": t["file"], "actions": t["actions"], + "has_description": bool(t["description"])} for t in topics], + "workflows": workflows, + "unreadable": [t["file"] for t in topics if "error" in t], + } + result["coverage_complete"] = not result["unreadable"] + + print(f"{len(topics)} topics, {len(workflows)} workflows\n") + for topic in topics: + acts = ", ".join(topic["actions"]) or "reply only" + print(f"- {topic['file']} [{acts}]") + if topic["description"]: + print(f" {topic['description']}") + if topic["triggers"]: + print(f" triggers: {'; '.join(topic['triggers'][:6])}") + if "error" in topic: + print(f" NOT analyzed - {topic['error']}") + if workflows: + print("\nWorkflows: " + ", ".join(workflows)) + if result["unreadable"]: + print(f"\n{len(result['unreadable'])} topic(s) NOT analyzed - " + "capability coverage is incomplete.") + + print(_SENTINEL + json.dumps(result)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/solutions/ess-maker-skills/src/reference/ess-docs/hardening/instruction-rules.md b/solutions/ess-maker-skills/src/reference/ess-docs/hardening/instruction-rules.md index ff033064..b208e3cd 100644 --- a/solutions/ess-maker-skills/src/reference/ess-docs/hardening/instruction-rules.md +++ b/solutions/ess-maker-skills/src/reference/ess-docs/hardening/instruction-rules.md @@ -10,10 +10,17 @@ over-committing answers — and for the contradictions that make any other rule ## How to use this -Every finding must quote the **exact line** it comes from. A finding you cannot anchor to a -specific line is not a finding — do not report it. Instructions are prose, so the temptation to -report a vague impression ("the tone section feels permissive") is high; resist it. If you cannot -point at the text, you cannot propose a diff for it. +Every finding must be anchored to a **specific sentence** of the maker's instructions. Instruction blocks +are typically a few enormous paragraphs, so anchoring to a physical line or pasting a whole paragraph +produces a report no one can review — quote the sentence, not the block it sits in. + +A finding you cannot anchor to *something concrete* is not a finding. Instructions are prose, so the +temptation to report a vague impression ("the tone section feels permissive") is high; resist it. + +**Missing safeguards are the exception, and they are common** — several categories below are defined by +absence. Anchor those to the **behavior** instead: state the specific thing that nothing constrains, and +where a rule would go. Never nominate an innocent sentence as the cause; a maker shown a "guilty" line that +did not permit the behavior will reasonably distrust the whole report. Two ideas govern the whole rule set: @@ -41,6 +48,11 @@ program. **Report both sides.** A contradiction has no single guilty line, and proposing a fix to only one half usually breaks the behavior the other half was protecting. +**Threshold.** Name a concrete request where the two rules demand different behavior and both cannot be +satisfied. Rules that merely pull in different directions — warmth and caution, brevity and completeness — +are the normal texture of instructions, not defects. Without this bar, a reviewer under pressure to produce +findings will classify ordinary tone-versus-grounding pairs as conflicts. + ### INSTR-002 — Scope collision A general rule whose plain reading swallows a legitimate case the agent must still handle. @@ -52,10 +64,14 @@ breath, when only the first is unwanted. ### INSTR-003 — Precedence gap -Two rules conflict and nothing says which wins. Any instruction meant to be absolute must say so -and say what it outranks. "Be helpful and warm" and "never state a fact you did not retrieve" -will collide constantly; without precedence, tone usually wins, because tone rules are stated -first and phrased more confidently. +Two rules that genuinely conflict, with nothing saying which wins. Any instruction meant to be absolute +must say so and say what it outranks. + +Apply the threshold above: tone guidance sitting alongside a grounding rule is **not** a precedence gap. +"Be helpful and warm" and "never state a fact you did not retrieve" are satisfiable together, and reporting +that pair is the most common false positive in this whole rule set. A real gap needs a request where +following one rule requires breaking the other — for example, instructions that require every answer to end +with a next step, alongside a rule forbidding suggestions the sources do not support. ### INSTR-004 — Unreachable or vacuous rule @@ -106,9 +122,12 @@ HR, benefits, payroll, and leave agents. ### INSTR-015 — Referral to external authorities -No rule prevents directing users to a government agency, regulator, or court. Distinguish -carefully from **endorsed** support resources named in approved content, which must stay allowed — -see INSTR-001. +No rule prevents directing users to a government agency, regulator, or court. + +**You cannot resolve this one from the instructions alone.** Approved content frequently *does* endorse +outside resources — an employee assistance program, a benefits carrier, an ombudsman — and a blanket ban +would contradict the organization's own material. This review cannot see the knowledge sources, so ask the +maker which external resources are deliberately endorsed and carve them out. Compare INSTR-001. --- @@ -127,10 +146,16 @@ itself the finding — the list is evidence the problem was recognized and the f ### INSTR-021 — Capability inference Nothing constrains the agent to actions its configured tools and topics actually support, so it -infers capability from what systems of that kind usually do. Because a maker's tool inventory -changes, prefer an abstract rule ("unless a configured tool supports it") over enumerating -specific actions the agent cannot perform — an enumeration goes stale and, worse, can prohibit -something the agent genuinely does support. +infers capability from what systems of that kind usually do. + +**Establish the real inventory before writing any rule here** — `scripts/list_agent_capabilities.py` +reports what each topic handles and whether it acts or only replies. Note that answering *about* +something and *doing* it are different capabilities: an agent that reports a leave balance may have no +way to submit a leave request. + +Because a maker's tool inventory changes, prefer an abstract rule ("unless a configured tool supports +it") over enumerating specific actions the agent cannot perform — an enumeration goes stale and, worse, +can prohibit something the agent genuinely does support. ### INSTR-022 — Standing helpfulness pressure @@ -176,7 +201,8 @@ instructions. Carve those out explicitly; they are not offers. When proposing a change: -- **Quote the line you are replacing.** Show old and new, never a summary of the change. +- **Quote the sentence you are replacing**, not the paragraph containing it. A small edit shown as a + 1,000-character before-and-after block is not reviewable, and makers will approve it without reading. - **Prefer removal to addition.** The budget is finite and a removed permissive line is often worth more than an added prohibition. Removing "always ready to help" does more for INSTR-020 than any new sentence. @@ -189,8 +215,26 @@ When proposing a change: - **Do not invent an escalation mechanism.** Use the agent's existing configured path. Naming a channel, link, or process the maker never configured is itself an ungrounded instruction. -## Part 6 — Product-default findings +## Part 6 — Shipped-template markers + +Most ESS instruction blocks start life as a shipped template, so many findings are inherited rather than +something the maker wrote. Saying so matters: an unqualified report reads as an accusation about text they +never authored. + +But **only claim template provenance when the wording actually matches**. There is no provenance field to +check, and guessing is how a report either blames a maker for product text or excuses text they wrote +themselves. These phrases appear verbatim in a shipped ESS HR default and are safe to attribute: + +- "Your tone and voice are warm and relaxed, crisp, and clear, and ready to lend a hand" — INSTR-022, and + INSTR-010 via "You provide authoritative and succinct answers" +- "Users trust you to provide them with relevant information from configured knowledge sources" +- "Do not try to provide answers when you don't have enough information" +- "You must not use your own general knowledge" + +Note what that default does **not** contain: any constraint on offering unsupported actions, on referring +users to outside authorities, or on jurisdiction scope. Those gaps (INSTR-014, INSTR-015, INSTR-020, +INSTR-021) are absences in the shipped baseline, not maker mistakes — and they are the findings most likely +to matter. -Instructions derived from a shipped ESS template commonly carry INSTR-010, INSTR-011, INSTR-013, -and INSTR-022 unmodified. Finding them is expected and is not evidence the maker did anything -wrong — say so when reporting, or the report reads as an accusation about text they never wrote. +Other templates differ. A derivative that sanctions "general guidance" carries INSTR-011, which the default +above does not. Check the text in front of you rather than assuming a variant. diff --git a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md index 05bcc1da..7d1ca1fc 100644 --- a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md +++ b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md @@ -13,33 +13,48 @@ the agent is behaving well: a rule contradicted elsewhere is not in force, howev > the maker approves. Instructions govern every answer the agent gives, so an unreviewed edit here is far > more dangerous than an unreviewed edit to a single topic. +**All paths in this skill are relative to the solution root** — the folder containing `scripts/`, `src/`, +and `workspace/`. They are not relative to this file. + ## Rules -- **Never rewrite the instructions wholesale.** Propose the smallest set of line-level changes that address - what was actually found. A rewrite is unreviewable — the maker cannot tell an intended change from an - incidental one — and it discards wording their organization may have chosen deliberately. +- **Never rewrite the instructions wholesale.** Propose the smallest set of changes that address what was + actually found. A rewrite is unreviewable — the maker cannot tell an intended change from an incidental + one — and it discards wording their organization may have chosen deliberately. - **Do not tighten just because you were invoked.** If the review finds nothing and the maker reports no problem, say so and stop. Adding prohibitions "to be safe" causes the agent to refuse questions its sources fully answer, which is a real regression traded for a hypothetical one. -- **Quote before proposing.** Every finding and every proposed change names the exact line it applies to. - Findings you cannot anchor to a line do not get reported. -- **Never propose a rule that blocks a supported action.** Check the agent's configured topics and tools - before prohibiting anything the agent might legitimately do (see `INSTR-032`). -- **Never apply changes without a checkpoint** (Step 7). +- **Anchor every finding to a sentence** — see "Anchoring" below. Never attribute a problem to a sentence + that did not cause it. +- **Never propose a rule that blocks a supported action.** Establish what the agent supports (Step 5) + before prohibiting anything. +- **Never apply changes without a checkpoint** (Step 9). - **Do not push.** This skill writes locally. Pushing is the maker's separate, explicit decision via `/push`. -- **Run the analysis silently.** Steps 3–5 are internal. Do not narrate which files you are reading, which - rule ids matched, or what you are about to check. The maker sees Step 6 onward. -- **Speak the maker's language.** Never show `INSTR-*` ids, rule-pack filenames, or the words "detector", - "rule pack", or "probe". Describe each finding in plain language and quote the maker's own text. Their - instruction wording is *their* language and should be shown in full. -- **TRACK PROGRESS**: use the todo list tool to track the steps below so the maker can see where you are. +- **Run the analysis silently.** Steps 3–5 are internal. Do not narrate which files you are reading, what + you are about to check, or what you found until Step 8. Keep progress labels generic — "Reviewing the + instructions" is fine, "Checking for jurisdiction-scope gaps" is not. +- **Speak the maker's language.** Never show `INSTR-*` ids, the filenames of this skill's own reference + material, or the words "detector", "rule pack", or "probe". Describe findings in plain language. The + maker's own instruction wording is *their* language and is shown verbatim. +- **TRACK PROGRESS**: use the todo list tool for the steps below. + +## Anchoring + +Instruction blocks are usually a handful of enormous paragraphs — a single "line" can run past 1,000 +characters. Quoting a whole paragraph to justify a nine-word change produces a proposal no one can review. + +- **Anchor to the sentence**, not the physical line or the paragraph. +- **Quote only the sentence you are changing**, plus at most a few words either side if the change would + otherwise be ambiguous. +- **For a missing safeguard, do not pick a "guilty" sentence.** Say plainly that nothing in the + instructions constrains the behavior, and name where the new rule would go — for example "in the same + paragraph as the existing restrictions". Inventing a culprit misleads the maker about their own text. ## What this checks and what it does not -This skill reads **only the instructions text**, plus the agent's topic and tool inventory for the -capability checks. It **cannot** tell you whether the agent actually produces a bad answer — instructions -are one input to that, and retrieval quality, knowledge-source coverage, and the underlying model matter at -least as much. +This skill reads the **instructions text** and the agent's **topic and workflow inventory**. It **cannot** +see the agent's knowledge sources, and it cannot tell you whether the agent actually produces a bad answer +— retrieval quality, knowledge-source coverage, and the underlying model matter at least as much. Say this plainly when it is relevant. A maker whose agent gives ungrounded answers because a knowledge source is not being retrieved will get no benefit from tighter instructions, and letting them believe @@ -47,15 +62,16 @@ otherwise costs them the time they should have spent on retrieval. Two signals t wrong lever: - the agent answers correctly when the maker pastes the source content into the chat, but not otherwise; -- the agent says it cannot find information that the maker knows is in an attached source. +- the agent says it cannot find information the maker knows is in an attached source. Both point at knowledge-source configuration. Route those makers to `/flightcheck` and `/troubleshoot` rather than editing instructions. ## Step 1: Resolve the agent -Read `.local/config.json` for the agent folder. The instructions live at -`workspace/agents/{agent.folder}/agent.mcs.yml` in the `instructions:` block. +Read `.local/config.json`. The active agent's folder is `agent.folder` — a path relative to the solution +root, e.g. `workspace/agents/`. The instructions are the `instructions:` block of +`{agent.folder}/agent.mcs.yml`. If the file is missing, tell the maker their agent has not been extracted yet and to run `/setup`, then STOP. @@ -78,34 +94,55 @@ and it determines whether Step 6 proposes changes or only reports. Record their answer. Do not paraphrase a vague answer into a specific complaint — if they said "it makes things up sometimes" without an example, you have a **theme**, not a case, and Step 6 treats those -differently. +differently. Wanting the agent "locked down" before a rollout is not a reported problem; it is branch B. -## Step 3: Read the instructions and the rule pack +## Step 3: Read the instructions and the reference guidance -Read the full `instructions:` value and the rule pack at -[`instruction-rules.md`](src/reference/ess-docs/hardening/instruction-rules.md). +Read the full `instructions:` value and `src/reference/ess-docs/hardening/instruction-rules.md`. -Split the instructions into numbered lines so every finding can be anchored precisely. Keep the maker's -original wording, spelling, and casing exactly — you will be quoting it back and later diffing against it. +Split the instructions into numbered **sentences** so findings can be anchored precisely (see "Anchoring"). +Keep the maker's original wording, spelling, and casing exactly — you will quote it back and diff against it. ## Step 4: Contradiction pass (always runs) -Apply Part 1 of the rule pack (`INSTR-001` … `INSTR-005`). This pass runs regardless of the maker's answer -in Step 2. +Apply Part 1 of the reference guidance (`INSTR-001` … `INSTR-005`). This pass runs regardless of the +maker's answer in Step 2. + +**Threshold.** Report a contradiction only when you can state a **concrete request** where the two rules +demand different behavior and both cannot be satisfied. Tone guidance and grounding rules coexisting is not +by itself a contradiction — "be warm and authoritative" and "don't answer without enough information" are +routinely satisfiable together. Without this bar you will manufacture a conflict to justify the run. -For each contradiction, record **both** conflicting lines. Do not decide which one is "right" — the maker -knows which behavior they intended, and guessing produces a fix that removes a guardrail they wanted. +For each contradiction, record **both** sentences. Do not decide which one is "right": the maker knows which +behavior they intended. In Step 8 you present the conflict and the options, and let them choose. ## Step 5: Grounding and over-commitment pass -Apply Parts 2 and 3 of the rule pack (`INSTR-010` … `INSTR-022`). +Apply Parts 2 and 3 of the reference guidance (`INSTR-010` … `INSTR-022`). + +Where the maker gave specific bad responses in Step 2, work backward from each one: identify which sentence +*permitted* it, or state that nothing constrains it. A missing safeguard is a valid finding — anchor it as +described under "Anchoring". -Where the maker gave specific bad responses in Step 2, work backward from each one: identify which -instruction *permitted* it, or which instruction that would have prevented it is **absent**. An absence is a -valid finding as long as you can state the specific behavior that is unconstrained. +**Establish what the agent supports before writing any capability rule.** Run: -For capability findings (`INSTR-021`), read the agent's topic files (`{agent.folder}/topics/`) and tool -inventory first. You need to know what the agent *can* do before writing a rule about what it cannot. +``` +python scripts/list_agent_capabilities.py --agent {agent.folder} +``` + +This lists every topic with what the model is told it handles and whether it merely replies or invokes a +flow, connector, or HTTP call, plus the workflow inventory. Use it — do not read the topic tree by hand. A +stock agent has dozens of topics, several of them large generated files, so reading them is both expensive +and unreliable, and a capability conclusion drawn from a sample is exactly how a supported action gets +prohibited. Open an individual topic only when the inventory is genuinely ambiguous about a capability you +are about to write a rule for. + +Note the distinction the inventory draws: a topic that *answers about* something is not the same as a topic +that *does* it. "Look up leave balance" and "submit a time-off request" are different capabilities. + +The inventory lists workflows by name only, and a name like "create case" does not reveal what the workflow +actually does or who calls it. If a capability rule depends on a workflow's behavior, ask the maker rather +than inferring it from the name. For any finding where an existing rule already targets the reported behavior **by listing forbidden phrases**, record that the mechanism itself failed. Phrase lists are evaded by rewording; the replacement @@ -121,23 +158,38 @@ either a reported behavior or a contradiction. Do not append unrelated hardening to be open. **B — the maker reported nothing specific.** -Propose fixes for **contradictions** (Step 4) and for findings where a rule is **internally inconsistent or +Propose fixes only for **contradictions** (Step 4) and for rules that are **internally inconsistent or vacuous** (`INSTR-004`, `INSTR-005`). Report the grounding and over-commitment findings from Step 5 as observations with the risk each carries, and ask whether the maker wants any of them addressed. Do not -propose those changes pre-approved. +propose those changes pre-approved. Reporting risks and proposing nothing is a **legitimate and complete +outcome** of this branch — it is not a failed run. + +Worth saying to the maker if they push back: prohibitions have a cost. Each one makes the agent more likely +to decline a question it could have answered, and without a reported problem there is nothing to weigh that +cost against. -The reason is worth stating to the maker if they push back: prohibitions have a cost. Each one makes the -agent more likely to decline a question it could have answered, and without a reported problem there is -nothing to weigh that cost against. +**Before finalizing any proposal**, check it against Part 4 of the reference guidance (`INSTR-030` … +`INSTR-033`): -Before finalizing any proposal, check it against Part 4 of the rule pack (`INSTR-030` … `INSTR-033`). Any -prohibition needs a stated alternative behavior, and the proposal as a whole needs a line stating the -prohibitions restrict invention rather than helpfulness. +- every prohibition states what the agent should do instead; +- the proposal as a whole includes a sentence stating the prohibitions restrict invention, not helpfulness; +- nothing prohibits a capability the Step 5 inventory shows the agent has. + +**One caveat you cannot resolve alone:** you cannot see the knowledge sources. If a proposed rule would ban +referrals to outside organizations, ask the maker whether any external resource — an employee assistance +program, a benefits carrier, an ombudsman — is deliberately endorsed in their content, and carve it out. +A blanket ban can contradict their own approved material. ## Step 7: Check the character budget -Instructions have a length ceiling in Copilot Studio, and hardening only adds text. Write the proposed full -instructions to `.local/harden/candidate.txt`, then run: +**If Step 6 produced no proposal, skip this step and go to Step 8.** There is nothing to measure, and +building a candidate identical to the current instructions only creates a file to clean up. + +Instructions have a length ceiling in Copilot Studio, and hardening usually lengthens them, so measure the +complete candidate rather than estimating the delta. + +Create `.local/harden/` if it does not exist, write the proposed **full** instructions to +`.local/harden/candidate.txt`, then run: ``` python scripts/check_instruction_budget.py --agent {agent.folder} --candidate .local/harden/candidate.txt @@ -147,28 +199,43 @@ Read the `###INSTRUCTION_BUDGET_JSON###` line. **Its verdict is authoritative yourself and do not override it.** - `ok` — proceed. -- `tight` — proceed, and tell the maker how little room is left. +- `tight` — proceed, and tell the maker how little room is left. Do **not** cut further wording just to + reach `ok`; deleting the maker's text to buy headroom they did not ask for is its own regression. - `over` — **do not present the proposal as-is.** Identify what to remove and propose that too. Prefer - removing permissive or vacuous lines (`INSTR-004`, `INSTR-011`, `INSTR-022`) — removing a line that invites - the bad behavior is usually worth more than the prohibition you were trying to add. Re-run until `ok`. + removing permissive or vacuous sentences (`INSTR-004`, `INSTR-011`, `INSTR-022`) — removing a sentence + that invites the bad behavior is usually worth more than the prohibition you were trying to add. Re-run + until the verdict is `ok` or `tight`. -The default limit is the kit's working assumption, not a verified platform constant. If the maker knows +The default limit is this kit's working assumption, not a verified platform constant. If the maker knows their real ceiling, pass it with `--limit`. +If you finish the run without applying anything — the maker declines, defers, or you had nothing to propose +— delete `.local/harden/candidate.txt`. A stale candidate is worse than none: a later run can measure or +present the wrong proposal. + ## Step 8: Present the proposal and get approval Present, in this order: -1. **What you found**, in plain language, grouped as contradictions first, then risks. Quote the maker's - own line for each. If the instructions came from a shipped template, say that where it applies — several - common findings are inherited defaults, not something the maker wrote. -2. **What you propose to change**, as before-and-after pairs: +1. **What you found**, in plain language, contradictions first, then risks. Quote the maker's own sentence + for each; for a missing safeguard, say nothing constrains the behavior rather than blaming a sentence. - > **Currently:** "{exact original line}" - > **Proposed:** "{exact replacement}" + Only attribute wording to the shipped template when it matches the marker text listed in Part 6 of the + reference guidance. Otherwise say nothing about where it came from — a wrong attribution either blames + the maker for product text or excuses text they wrote themselves. + +2. **What you propose to change**, as before-and-after pairs at sentence granularity: + + > **Currently:** "{the exact sentence being changed}" + > **Proposed:** "{the exact replacement}" > **Why:** {one or two sentences tied to what this prevents} - For a removal, show the line and say it is being removed and what that changes. + For a removal, quote the sentence and say what removing it changes. For an addition, quote nothing — + show the new sentence and say where it goes. **Never paste an entire paragraph** to show a small edit. + + For a contradiction with no obvious winner, present both directions as options and ask which behavior + they intended, rather than choosing for them. + 3. **The length**, in one line: the new total and the remaining headroom. 4. **What this does not cover**: instructions do not fix a knowledge source the agent cannot retrieve. If the maker's reported examples looked like retrieval problems (see "What this checks"), say so here. @@ -176,6 +243,8 @@ Present, in this order: Then ask for approval. The maker may accept all, accept some, or decline. Apply exactly what they accept. If there is nothing to propose, say so directly and stop — do not manufacture a finding to justify the run. +If you have risks but nothing to propose (branch B), present the risks, ask whether they want any addressed, +and stop there; that is a complete outcome. ## Step 9: Apply @@ -189,16 +258,19 @@ python scripts/emit_capability.py harden The `emit_capability.py` line records anonymous usage telemetry (best-effort, non-blocking); it needs no user-facing message and never fails the step. -Then edit the `instructions:` block in `workspace/agents/{agent.folder}/agent.mcs.yml`, changing only the -approved lines. Preserve the YAML block scalar style and the indentation of the surrounding file. +Then edit the `instructions:` block in `{agent.folder}/agent.mcs.yml`, changing only the approved sentences. +Preserve the YAML block scalar style and the indentation of the surrounding file. Preserve **exactly** any `{System.Bot.Components.Topics...}` references or other placeholder tokens in the instructions. These are live references, not example text — rewording one silently breaks the behavior it drives. -Re-run the budget check without `--candidate` to confirm the written file measures as expected. +If the maker accepted only part of the proposal, rebuild the candidate from what they accepted and re-run +the Step 7 check before writing. The measurement of a proposal they did not take does not describe the file +you are about to write. -Delete `.local/harden/candidate.txt` once applied. +Re-run the budget check without `--candidate` to confirm the written file measures as expected, then delete +`.local/harden/candidate.txt`. ## Step 10: Hand off to validation @@ -221,6 +293,7 @@ questions — and that a few normal, in-scope questions are worth testing alongs ## References -- [`instruction-rules.md`](src/reference/ess-docs/hardening/instruction-rules.md) — the rule pack: - contradiction classes, grounding and over-commitment risks, over-restriction risks, and rewriting - principles. +- `src/reference/ess-docs/hardening/instruction-rules.md` — contradiction classes, grounding and + over-commitment risks, over-restriction risks, rewriting principles, and the shipped-template markers. +- `scripts/list_agent_capabilities.py` — topic and workflow inventory (Step 5). +- `scripts/check_instruction_budget.py` — character-budget measurement (Step 7). diff --git a/tests/scripts/test_agent_capabilities.py b/tests/scripts/test_agent_capabilities.py new file mode 100644 index 00000000..488ab330 --- /dev/null +++ b/tests/scripts/test_agent_capabilities.py @@ -0,0 +1,196 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +""" +Unit tests for ``list_agent_capabilities`` — the inventory probe the +``/harden`` skill uses before writing any rule about what an agent cannot do. + +The rule this protects: never prohibit something the agent actually supports. +That check is only as good as the capability picture behind it, and the picture +has to come from somewhere cheaper than reading three dozen topic files, or it +gets skipped. + +The load-bearing case is ``test_description_survives_non_yaml_topic``. Copilot +Studio topic files are *not* valid YAML — they contain unquoted ``@type:`` keys +— and the files that fail a strict parse are exactly the Workday and ServiceNow +integration topics whose descriptions matter most. A parser-based +implementation silently returns "no description" for them, which reads as "the +agent can't do this" and invites a prohibition on a supported action. +""" + +from __future__ import annotations + +import json +import shutil +import subprocess +import sys +import uuid +from pathlib import Path + +import pytest + +import list_agent_capabilities as inventory + +_SKILL_ROOT = Path(inventory.__file__).resolve().parent.parent +_SCRIPT = _SKILL_ROOT / "scripts" / "list_agent_capabilities.py" +_AGENTS_DIR = _SKILL_ROOT / "workspace" / "agents" + +_SENTINEL = "###AGENT_CAPABILITIES_JSON###" + +# A topic shaped like a real generated integration topic: an unquoted "@type" +# nested in the body, which makes the file unparseable as strict YAML. +_NON_YAML_TOPIC = """\ +kind: AdaptiveDialog +modelDescription: You will respond to requests about the base compensation of the user. +beginDialog: + kind: OnRecognizedIntent + actions: + - kind: InvokeFlowAction + input: + binding: + value: + @type: String +""" + +_REPLY_ONLY_TOPIC = """\ +kind: AdaptiveDialog +modelDescription: | + You will respond only to questions about parking. + Do not answer anything else. +triggerQueries: + - where do I park + - parking policy +beginDialog: + kind: OnRecognizedIntent + actions: + - kind: SendActivity + activity: Here is the parking policy. +""" + + +@pytest.fixture +def temp_agent(): + """A throwaway agent with one acting topic, one reply-only topic, and one + undecodable file. Lives under the gitignored workspace/agents/ tree.""" + name = f"_pytest_{uuid.uuid4().hex}" + agent_dir = _AGENTS_DIR / name + topics = agent_dir / "topics" + topics.mkdir(parents=True) + (agent_dir / "workflows").mkdir() + + (topics / "workday-get-basecompensation.mcs.yml").write_text( + _NON_YAML_TOPIC, encoding="utf-8" + ) + (topics / "parking.mcs.yml").write_text(_REPLY_ONLY_TOPIC, encoding="utf-8") + (topics / "broken.mcs.yml").write_bytes(b"\xff\xfe\x00\x80\x81\xff") + (agent_dir / "workflows" / "workday-abc123").mkdir() + + try: + yield name + finally: + shutil.rmtree(agent_dir, ignore_errors=True) + + +def _run(*args) -> tuple[int, dict, str]: + proc = subprocess.run( + [sys.executable, str(_SCRIPT), *args], + capture_output=True, + text=True, + encoding="utf-8", + ) + combined = proc.stdout + proc.stderr + lines = [ln for ln in proc.stdout.splitlines() if ln.startswith(_SENTINEL)] + assert len(lines) == 1, f"expected one sentinel line, got {len(lines)}: {combined}" + return proc.returncode, json.loads(lines[0][len(_SENTINEL):]), combined + + +def _topic(result: dict, filename: str) -> dict: + match = [t for t in result["topics"] if t["file"] == filename] + assert match, f"{filename} missing from inventory: {result['topics']}" + return match[0] + + +def test_description_survives_non_yaml_topic(temp_agent): + """The regression that motivates this probe: an unquoted `@type` must not + cost us the topic's description.""" + _, result, combined = _run("--agent", temp_agent) + topic = _topic(result, "workday-get-basecompensation.mcs.yml") + + assert topic["has_description"] is True + assert "base compensation" in combined + assert "error" not in topic + + +def test_acting_topics_are_distinguished_from_reply_only(temp_agent): + """"Can answer about X" and "can do X" need different instruction rules, so + the inventory has to tell them apart.""" + _, result, _ = _run("--agent", temp_agent) + + assert _topic(result, "workday-get-basecompensation.mcs.yml")["actions"] == ["flow"] + assert _topic(result, "parking.mcs.yml")["actions"] == [] + + +def test_block_scalar_description_and_triggers(temp_agent): + _, _, combined = _run("--agent", temp_agent) + + assert ( + "You will respond only to questions about parking. Do not answer anything else." + in combined + ) + assert "triggers: where do I park; parking policy" in combined + + +def test_json_stays_compact(temp_agent): + """Descriptions belong in the table, not duplicated into the JSON. On a + real agent that duplication pushed the output past the caller's limit and + the capability evidence was lost.""" + _, result, _ = _run("--agent", temp_agent) + + assert "description" not in result["topics"][0] + assert set(result["topics"][0]) == {"file", "actions", "has_description"} + + +def test_unreadable_topic_is_disclosed(temp_agent): + """A topic that could not be read is a hole in the capability picture. The + caller is about to write prohibitions from that picture, so silence here + would be worse than the missing data.""" + code, result, combined = _run("--agent", temp_agent) + + assert code == 0, combined + assert "broken.mcs.yml" in result["unreadable"] + assert result["coverage_complete"] is False + assert "NOT analyzed" in combined + assert "coverage is incomplete" in combined + + +def test_workflows_are_listed(temp_agent): + _, result, _ = _run("--agent", temp_agent) + assert result["workflows"] == ["workday-abc123"] + + +def test_long_descriptions_are_truncated(temp_agent): + topics = _AGENTS_DIR / temp_agent / "topics" + (topics / "verbose.mcs.yml").write_text( + "kind: AdaptiveDialog\nmodelDescription: " + ("word " * 400) + "\n", + encoding="utf-8", + ) + _, _, combined = _run("--agent", temp_agent) + + description_line = [ + ln for ln in combined.splitlines() if ln.strip().startswith("word word") + ] + assert description_line, combined + assert description_line[0].strip().endswith("...") + assert len(description_line[0].strip()) <= inventory._MAX_DESCRIPTION + 8 + + +def test_agent_accepts_slug_or_config_folder_path(temp_agent): + _, by_slug, _ = _run("--agent", temp_agent) + _, by_path, _ = _run("--agent", f"workspace/agents/{temp_agent}") + assert by_slug["topic_count"] == by_path["topic_count"] + + +def test_missing_agent_is_reported(temp_agent): + code, result, _ = _run("--agent", "_pytest_does_not_exist") + assert code != 0 + assert "error" in result diff --git a/tests/scripts/test_instruction_budget.py b/tests/scripts/test_instruction_budget.py index 5695fd58..d5bcf167 100644 --- a/tests/scripts/test_instruction_budget.py +++ b/tests/scripts/test_instruction_budget.py @@ -16,6 +16,7 @@ from __future__ import annotations import json +import re import shutil import subprocess import sys @@ -156,6 +157,17 @@ def test_custom_limit_is_honoured(temp_agent): # error paths never produce a falsely clean verdict # --------------------------------------------------------------------------- # +def test_agent_accepts_slug_or_config_folder_path(temp_agent): + """`.local/config.json` stores `agent.folder` as `workspace/agents/` + and `activeAgent` as a bare slug. Both must resolve, or which one the + caller reaches for first decides whether the probe works.""" + _, by_slug, _ = _run("--agent", temp_agent) + _, by_path, _ = _run("--agent", f"workspace/agents/{temp_agent}") + + assert by_slug["chars"] == by_path["chars"] + assert by_path["verdict"] == "ok" + + def test_missing_agent_reports_unknown(): code, result, _ = _run("--agent", "_pytest_does_not_exist") assert code != 0 @@ -240,4 +252,9 @@ def test_harden_prompt_and_skill_are_present(): skill_text = skill.read_text(encoding="utf-8") assert "src/reference/ess-docs/hardening/instruction-rules.md" in skill_text assert "scripts/check_instruction_budget.py" in skill_text + assert "scripts/list_agent_capabilities.py" in skill_text assert "emit_capability.py harden" in skill_text + # The checkpoint rule is safety-critical and previously pointed at the + # wrong step; a stale cross-reference here is a real defect. + assert re.search(r"checkpoint\*{0,2} \(Step 9\)", skill_text), \ + "the checkpoint rule must cross-reference the step that actually runs it" From 5bbcf702618056deebeafa8e8e057cf3493644ca Mon Sep 17 00:00:00 2001 From: Avery Milandin Date: Mon, 24 Aug 2026 19:19:22 -0700 Subject: [PATCH 03/10] fix(harden): fix intake, validation handoff, and candidate contradiction check Three defects from running the skill against a live agent. - The intake question was answered with a menu of categories, and a picked category cannot be anchored to a change. Step 2 now forbids offering options when eliciting what the maker has seen, and requires a prose follow-up when the answer names a category rather than a behavior. - Runs ended on the diff without mentioning validation. Step 10 now runs on every path -- applied, declined, and nothing-proposed -- because reading instructions cannot show that the agent's answers improved. Step 8 no longer tells the model to stop. - Contradictions were only checked against the maker's existing text. Hardening adds prohibitions to a document that already has rules, so Step 6 now re-runs the contradiction pass against the whole candidate, and requires amending a colliding surviving rule rather than layering a stricter rule on top of it. Also: a branch-B maker who asks for a reported risk to be addressed now has a path back into the proposal steps, the Step 1 hard gates are carved out of the always-hand-off rule, and anchoring covers bullets and fragments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 96774230-f664-4887-baf1-a5bbfbd6cbb2 --- .../src/skills/instructions/harden/SKILL.md | 94 ++++++++++++++++--- tests/scripts/test_instruction_budget.py | 43 +++++++++ 2 files changed, 126 insertions(+), 11 deletions(-) diff --git a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md index 7d1ca1fc..c8ca0ad3 100644 --- a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md +++ b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md @@ -22,12 +22,22 @@ and `workspace/`. They are not relative to this file. actually found. A rewrite is unreviewable — the maker cannot tell an intended change from an incidental one — and it discards wording their organization may have chosen deliberately. - **Do not tighten just because you were invoked.** If the review finds nothing and the maker reports no - problem, say so and stop. Adding prohibitions "to be safe" causes the agent to refuse questions its - sources fully answer, which is a real regression traded for a hypothetical one. + problem, say so and propose nothing (you still finish with Step 10). Adding prohibitions "to be safe" + causes the agent to refuse questions its sources fully answer, which is a real regression traded for a + hypothetical one. - **Anchor every finding to a sentence** — see "Anchoring" below. Never attribute a problem to a sentence that did not cause it. - **Never propose a rule that blocks a supported action.** Establish what the agent supports (Step 5) before prohibiting anything. +- **Ask questions in prose, not as menus.** When you are eliciting what the maker has seen or wants + (Step 2), never present numbered or lettered options to pick from. A picked option is a category, and + categories are not evidence — you cannot anchor a change to one. Ask the open question and wait for their + own words. Offering a choice between concrete alternatives you have already drafted (Step 8) is fine; + that is a decision, not an interview. +- **Never end the run without Step 10.** Every path that reached the analysis — applied, declined, nothing + found — ends by pointing at `/test` and `/evaluate`. This skill reads text; it cannot show that the + agent's answers improved. The only exceptions are the Step 1 gates, where there is no agent or no + instructions to review and the maker is sent to `/setup` instead. - **Never apply changes without a checkpoint** (Step 9). - **Do not push.** This skill writes locally. Pushing is the maker's separate, explicit decision via `/push`. - **Run the analysis silently.** Steps 3–5 are internal. Do not narrate which files you are reading, what @@ -44,6 +54,8 @@ Instruction blocks are usually a handful of enormous paragraphs — a single "li characters. Quoting a whole paragraph to justify a nine-word change produces a proposal no one can review. - **Anchor to the sentence**, not the physical line or the paragraph. +- **A bullet, a heading, or a standalone fragment counts as one unit** even when it is not a grammatical + sentence. Split on what the maker would recognize as a single rule, not on punctuation. - **Quote only the sentence you are changing**, plus at most a few words either side if the change would otherwise be ambiguous. - **For a missing safeguard, do not pick a "guilty" sentence.** Say plainly that nothing in the @@ -83,6 +95,10 @@ usually means the agent's instructions were never configured rather than that th Ask before analyzing. What the maker has observed is better evidence than anything derivable from the text, and it determines whether Step 6 proposes changes or only reports. +**Ask in plain prose. Do not offer numbered options, lettered choices, or a menu to pick from.** The +answer you need is the maker's own description of what went wrong; a menu invites them to pick a category +instead, and a category tells you nothing you can anchor a change to. Ask this as a single open question: + > Before I look at your instructions — have you seen specific answers from your agent that you didn't like? > > If you can paste one or two, that helps most: the exact question and what the agent said. Otherwise, tell @@ -92,6 +108,12 @@ and it determines whether Step 6 proposes changes or only reports. > If nothing specific has gone wrong, that's fine too — say so and I'll check the instructions for > contradictions and gaps and tell you what I find. +**If the answer names a category rather than a behavior, you do not have an answer yet — ask again.** +"General concerns", "the usual problems", "hallucination", or picking one of your own examples back are +labels, not evidence. Follow up in prose — *"What has it been doing that concerns you?"* — and wait. Do +not proceed to Step 3 on a label. A proposal built from a category is a proposal built from nothing, and +it will read as generic hardening because that is what it is. + Record their answer. Do not paraphrase a vague answer into a specific complaint — if they said "it makes things up sometimes" without an example, you have a **theme**, not a case, and Step 6 treats those differently. Wanting the agent "locked down" before a rollout is not a reported problem; it is branch B. @@ -105,8 +127,12 @@ Keep the maker's original wording, spelling, and casing exactly — you will quo ## Step 4: Contradiction pass (always runs) -Apply Part 1 of the reference guidance (`INSTR-001` … `INSTR-005`). This pass runs regardless of the -maker's answer in Step 2. +Apply Part 1 of the reference guidance (`INSTR-001` … `INSTR-005`) to the **current** instructions. This +pass runs regardless of the maker's answer in Step 2, and it runs here — before any proposal exists — so +that what you find describes the maker's text rather than your own. + +A second contradiction pass runs in Step 6 against the proposed text. Hardening adds prohibitions to a +document that already has rules, which is precisely how contradictions get created. **Threshold.** Report a contradiction only when you can state a **concrete request** where the two rules demand different behavior and both cannot be satisfied. Tone guidance and grounding rules coexisting is not @@ -164,15 +190,32 @@ observations with the risk each carries, and ask whether the maker wants any of propose those changes pre-approved. Reporting risks and proposing nothing is a **legitimate and complete outcome** of this branch — it is not a failed run. +**A theme is branch A, narrowly.** "It makes things up about benefits" or "it offers to do things it +can't" names a behavior class without an example. Treat it as branch A but scope every change to that +class, and say in Step 8 which findings you addressed because they match the theme and which you are only +reporting. Without an example you cannot confirm the instructions caused it, so present the change as your +best reading rather than a diagnosis. + Worth saying to the maker if they push back: prohibitions have a cost. Each one makes the agent more likely to decline a question it could have answered, and without a reported problem there is nothing to weigh that cost against. -**Before finalizing any proposal**, check it against Part 4 of the reference guidance (`INSTR-030` … -`INSTR-033`): +**Before finalizing any proposal**, run the Step 4 contradiction pass again — this time against the +**candidate as a whole**: your new and amended sentences read together with every original sentence you are +leaving in place. Use the same threshold. A new prohibition frequently collides with a rule that stays +behind: "never offer a next step" against an existing "always end by offering further help", or "answer +only from retrieved content" against an existing instruction to fall back on general knowledge. + +When the candidate contradicts a surviving rule, **amend or remove that rule as part of the same +proposal**. Do not stack a stricter rule on top and rely on it winning — that is the failure this skill +exists to catch, and you would be introducing it. Show the surviving rule you changed in Step 8 alongside +the rest, so the maker approves that removal explicitly rather than discovering it later. + +Then check the proposal against Part 4 of the reference guidance (`INSTR-030` … `INSTR-033`): - every prohibition states what the agent should do instead; -- the proposal as a whole includes a sentence stating the prohibitions restrict invention, not helpfulness; +- the proposal as a whole includes a sentence stating the prohibitions restrict invention, not helpfulness + — this sentence is part of proposing safely and is required even in a narrowly scoped theme run; - nothing prohibits a capability the Step 5 inventory shows the agent has. **One caveat you cannot resolve alone:** you cannot see the knowledge sources. If a proposed rule would ban @@ -242,9 +285,14 @@ Present, in this order: Then ask for approval. The maker may accept all, accept some, or decline. Apply exactly what they accept. -If there is nothing to propose, say so directly and stop — do not manufacture a finding to justify the run. -If you have risks but nothing to propose (branch B), present the risks, ask whether they want any addressed, -and stop there; that is a complete outcome. +If there is nothing to propose, say so directly — do not manufacture a finding to justify the run — and go +to Step 10. If you have risks but nothing to propose (branch B), present the risks, ask whether they want +any addressed, and go to Step 10; that is a complete outcome. Do not end the conversation here on any path. + +**If a branch-B maker asks for one of the reported risks to be addressed**, that risk becomes a reported +problem. Return to Step 6 and draft a proposal for the risks they named and nothing else, re-run the +candidate contradiction check and Step 7, and come back here to present it. Do not draft it in the same +message you asked the question in — you would be pre-approving the change you just said you would not. ## Step 9: Apply @@ -274,8 +322,14 @@ Re-run the budget check without `--candidate` to confirm the written file measur ## Step 10: Hand off to validation +**This step always runs — after applying, after a decline, and after a run that proposed nothing.** It is +the last thing the maker sees. Do not end the conversation on a diff, on "done", or on a summary of +findings. + Instruction changes are behavioral changes, and this skill has no way to demonstrate that the new text -produces better answers. Say that plainly and route the maker onward: +produces better answers. Say that plainly and route the maker onward. + +**If changes were applied:** > These changes aren't verified yet — instructions affect every answer, so it's worth checking the agent > still behaves the way you want. @@ -291,6 +345,24 @@ evaluation rows available, and they are the only direct evidence of whether this Also mention, once, that a change intended to prevent a bad answer can also cause the agent to decline good questions — and that a few normal, in-scope questions are worth testing alongside the failing ones. +**If the maker declined or deferred the proposal:** + +> Nothing has changed. If you want to see whether the behaviour I described actually shows up: +> +> - `/test` — try the agent directly +> - `/evaluate` — build test cases so you have a baseline before changing anything + +**If nothing was proposed:** + +> I didn't find anything worth changing in the instructions. That doesn't mean the agent answers well — +> instructions are only one input, and knowledge sources and topics matter at least as much. +> +> - `/test` — try the agent directly +> - `/evaluate` — build a test set, which will find behaviour problems this review cannot see + +Reading the instructions cannot tell you what the agent actually says. Do not let a clean review stand as +evidence that the agent is fine. + ## References - `src/reference/ess-docs/hardening/instruction-rules.md` — contradiction classes, grounding and diff --git a/tests/scripts/test_instruction_budget.py b/tests/scripts/test_instruction_budget.py index d5bcf167..2b277f06 100644 --- a/tests/scripts/test_instruction_budget.py +++ b/tests/scripts/test_instruction_budget.py @@ -258,3 +258,46 @@ def test_harden_prompt_and_skill_are_present(): # wrong step; a stale cross-reference here is a real defect. assert re.search(r"checkpoint\*{0,2} \(Step 9\)", skill_text), \ "the checkpoint rule must cross-reference the step that actually runs it" + + +def test_skill_always_routes_to_validation(): + """A real run ended on the diff and never mentioned validation. Reading + instructions cannot show that the agent's answers improved, so every + path has to hand off.""" + skill = _SKILL_ROOT / "src" / "skills" / "instructions" / "harden" / "SKILL.md" + skill_text = skill.read_text(encoding="utf-8") + + assert "Never end the run without Step 10." in skill_text + assert "`/evaluate`" in skill_text + assert "`/test`" in skill_text + # Step 8 previously told the model to "stop" on the no-proposal paths, + # which is what skipped the handoff. + step_8 = skill_text.split("## Step 8:")[1].split("## Step 9:")[0] + assert "go to Step 10" in step_8 + assert "and stop" not in step_8 + + +def test_skill_rechecks_contradictions_in_the_proposal(): + """Step 4 reads the maker's text. Hardening then adds prohibitions to a + document that already has rules, so the candidate has to be checked too — + otherwise the skill introduces the defect it exists to find.""" + skill = _SKILL_ROOT / "src" / "skills" / "instructions" / "harden" / "SKILL.md" + step_6 = (skill.read_text(encoding="utf-8") + .split("## Step 6:")[1].split("## Step 7:")[0]) + + assert "run the Step 4 contradiction pass again" in step_6 + assert "candidate as a whole" in step_6 + # The fix for a collision is to amend the surviving rule, not to layer a + # stricter one on top and hope it wins. + assert "amend or remove that rule" in step_6.lower() + + +def test_skill_forbids_option_menus_at_intake(): + """A maker offered a menu picks a category, and a category cannot be + anchored to a change — the run then produces generic hardening.""" + skill = _SKILL_ROOT / "src" / "skills" / "instructions" / "harden" / "SKILL.md" + step_2 = (skill.read_text(encoding="utf-8") + .split("## Step 2:")[1].split("## Step 3:")[0]) + + assert "Do not offer numbered options" in step_2 + assert "ask again" in step_2 From 0c93e1759d55f6c64e64ca1f43210de1b424f233 Mon Sep 17 00:00:00 2001 From: Avery Milandin Date: Mon, 24 Aug 2026 19:28:24 -0700 Subject: [PATCH 04/10] fix(harden): say changes are local, and route validation correctly - Applying edited agent.mcs.yml and reported it as done, which reads as live. Step 9 now states that Copilot Studio still has the previous instructions until the maker runs /push. - Step 10 routed makers to /test to check an instruction change. /test drives a topic or workflow and does not exercise system instructions, and /evaluate authors cases for the Copilot Studio Evaluation portal, which runs against the agent as deployed. Neither reads the local agent.mcs.yml, so the push has to come first or the evaluation measures the old text. The handoff now says so and orders the steps accordingly. - The progress list was created and never updated. Adopt the fuller house wording used by /cleanup: mark each step in-progress and complete as it happens, which matters here because several steps wait on the maker. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 96774230-f664-4887-baf1-a5bbfbd6cbb2 --- .../src/skills/instructions/harden/SKILL.md | 47 ++++++++++++++----- tests/scripts/test_instruction_budget.py | 28 +++++++++++ 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md index c8ca0ad3..a9cb5bf3 100644 --- a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md +++ b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md @@ -46,7 +46,11 @@ and `workspace/`. They are not relative to this file. - **Speak the maker's language.** Never show `INSTR-*` ids, the filenames of this skill's own reference material, or the words "detector", "rule pack", or "probe". Describe findings in plain language. The maker's own instruction wording is *their* language and is shown verbatim. -- **TRACK PROGRESS**: use the todo list tool for the steps below. +- **TRACK PROGRESS**: Use the todo list tool to track your progress through this skill's steps. Create a + todo list at the start with all the steps, mark each in-progress as you start it, and mark completed when + done. Update it as you go rather than at the end — several steps here wait on the maker's reply, and a + list that never moves gives them no idea whether you are working or waiting. If you loop back (Step 8 to + Step 6), reopen that step rather than leaving it complete. ## Anchoring @@ -320,6 +324,13 @@ you are about to write. Re-run the budget check without `--candidate` to confirm the written file measures as expected, then delete `.local/harden/candidate.txt`. +**Say where the change landed.** "I updated `agent.mcs.yml`" reads as done, and a maker who believes the +change is live will stop watching for the behavior they reported. Be explicit that this is a local file and +that the agent in Copilot Studio still has the old instructions: + +> I've updated your local copy of the agent. Your agent in Copilot Studio is still running the previous +> instructions — the change reaches it when you run `/push`. + ## Step 10: Hand off to validation **This step always runs — after applying, after a decline, and after a run that proposed nothing.** It is @@ -329,36 +340,46 @@ findings. Instruction changes are behavioral changes, and this skill has no way to demonstrate that the new text produces better answers. Say that plainly and route the maker onward. +**Know what the two commands actually do before you route someone to them.** `/evaluate` *authors* +evaluation cases; the CSVs land in `workspace/tests/{date}/` and are uploaded to the Copilot Studio +Evaluation portal, which runs them against the agent **as deployed**. `/test` drives a **topic or a +workflow** to debug its runtime behavior — it does not exercise system instructions, so it is not the way +to check an instruction change. Neither command reads the local `agent.mcs.yml`, which means an instruction +change has to be pushed before any of this can observe it. + **If changes were applied:** -> These changes aren't verified yet — instructions affect every answer, so it's worth checking the agent -> still behaves the way you want. +> Two things to know before you check whether this worked. > -> - `/evaluate` — turn the answers you didn't like into test cases, so you can tell whether this fixed them -> - `/test` — try the agent's behaviour directly -> - `/push` — send the change to Copilot Studio when you're ready +> The change is in your local copy only — your agent in Copilot Studio is still running the previous +> instructions. Run `/push` to send it. +> +> After that, `/evaluate` will turn the answers you didn't like into evaluation cases. It writes a CSV you +> upload to the Copilot Studio Evaluation portal, which runs them against your agent and shows you whether +> the behaviour actually changed. Where the maker gave specific bad responses in Step 2, carry them forward: those are the highest-value evaluation rows available, and they are the only direct evidence of whether this pass worked. Offer to run `/evaluate` with them. Also mention, once, that a change intended to prevent a bad answer can also cause the agent to decline good -questions — and that a few normal, in-scope questions are worth testing alongside the failing ones. +questions — so the evaluation set should include a few normal, in-scope questions alongside the failing +ones. Without those rows, an agent that has started refusing everything still scores clean. **If the maker declined or deferred the proposal:** -> Nothing has changed. If you want to see whether the behaviour I described actually shows up: -> -> - `/test` — try the agent directly -> - `/evaluate` — build test cases so you have a baseline before changing anything +> Nothing has changed, locally or in Copilot Studio. If you want to find out whether the behaviour I +> described actually shows up, `/evaluate` will build an evaluation set you can run against the agent as +> it stands — that gives you a baseline before changing anything. **If nothing was proposed:** > I didn't find anything worth changing in the instructions. That doesn't mean the agent answers well — > instructions are only one input, and knowledge sources and topics matter at least as much. > -> - `/test` — try the agent directly -> - `/evaluate` — build a test set, which will find behaviour problems this review cannot see +> `/evaluate` will build an evaluation set to run against the agent, which finds behaviour problems this +> review cannot see. If you suspect one specific topic or workflow is misbehaving, `/test` drives that +> component directly. Reading the instructions cannot tell you what the agent actually says. Do not let a clean review stand as evidence that the agent is fine. diff --git a/tests/scripts/test_instruction_budget.py b/tests/scripts/test_instruction_budget.py index 2b277f06..620afb5d 100644 --- a/tests/scripts/test_instruction_budget.py +++ b/tests/scripts/test_instruction_budget.py @@ -292,6 +292,34 @@ def test_skill_rechecks_contradictions_in_the_proposal(): assert "amend or remove that rule" in step_6.lower() +def test_skill_states_changes_are_local_until_push(): + """A maker told 'I updated agent.mcs.yml' reasonably concludes the change + is live, stops watching for the behaviour, and never pushes.""" + skill = _SKILL_ROOT / "src" / "skills" / "instructions" / "harden" / "SKILL.md" + skill_text = skill.read_text(encoding="utf-8") + step_9 = skill_text.split("## Step 9:")[1].split("## Step 10:")[0] + + assert "still running the previous" in step_9 + assert "`/push`" in step_9 + + # /evaluate uploads to the Copilot Studio Evaluation portal and /test + # drives a topic or workflow -- neither reads the local agent.mcs.yml, so + # the push has to come first or the evaluation measures the old text. + step_10 = skill_text.split("## Step 10:")[1].split("## References")[0] + assert "as deployed" in step_10 + assert "does not exercise system instructions" in step_10 + + +def test_skill_requires_live_progress_tracking(): + """The list was created and then never updated, so the maker could not + tell whether the run was working or waiting on them.""" + skill = _SKILL_ROOT / "src" / "skills" / "instructions" / "harden" / "SKILL.md" + skill_text = skill.read_text(encoding="utf-8") + + assert "mark each in-progress as you start it" in skill_text + assert "Update it as you go rather than at the end" in skill_text + + def test_skill_forbids_option_menus_at_intake(): """A maker offered a menu picks a category, and a category cannot be anchored to a change — the run then produces generic hardening.""" From 11d4afbf8fbd3ea18ddfe7cbb019d1fca3f3f007 Mon Sep 17 00:00:00 2001 From: Avery Milandin Date: Mon, 24 Aug 2026 19:35:35 -0700 Subject: [PATCH 05/10] fix(harden): stop leaking command-routing logic to the maker The handoff told makers that /test is not for system instructions. That is this skill's routing logic, not a next step -- the maker asked what to do and got a disqualification. Mark the explanation internal, recommend only the command that fits, and add a rule covering the general case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 96774230-f664-4887-baf1-a5bbfbd6cbb2 --- .../src/skills/instructions/harden/SKILL.md | 16 ++++++++++------ tests/scripts/test_instruction_budget.py | 8 ++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md index a9cb5bf3..08eaedcb 100644 --- a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md +++ b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md @@ -46,6 +46,9 @@ and `workspace/`. They are not relative to this file. - **Speak the maker's language.** Never show `INSTR-*` ids, the filenames of this skill's own reference material, or the words "detector", "rule pack", or "probe". Describe findings in plain language. The maker's own instruction wording is *their* language and is shown verbatim. +- **Recommend, don't disqualify.** Name the command that fits and stop. Do not tell the maker which + commands are *not* right for their situation, or why — that is this skill's routing logic, not their + next step, and it reads as hedging. - **TRACK PROGRESS**: Use the todo list tool to track your progress through this skill's steps. Create a todo list at the start with all the steps, mark each in-progress as you start it, and mark completed when done. Update it as you go rather than at the end — several steps here wait on the maker's reply, and a @@ -340,12 +343,13 @@ findings. Instruction changes are behavioral changes, and this skill has no way to demonstrate that the new text produces better answers. Say that plainly and route the maker onward. -**Know what the two commands actually do before you route someone to them.** `/evaluate` *authors* -evaluation cases; the CSVs land in `workspace/tests/{date}/` and are uploaded to the Copilot Studio -Evaluation portal, which runs them against the agent **as deployed**. `/test` drives a **topic or a -workflow** to debug its runtime behavior — it does not exercise system instructions, so it is not the way -to check an instruction change. Neither command reads the local `agent.mcs.yml`, which means an instruction -change has to be pushed before any of this can observe it. +**Internal — do not say any of this to the maker.** `/evaluate` *authors* evaluation cases; the CSVs land +in `workspace/tests/{date}/` and are uploaded to the Copilot Studio Evaluation portal, which runs them +against the agent **as deployed**. `/test` drives a **topic or a workflow** to debug its runtime behavior; +it does not exercise system instructions, so it is not the way to check an instruction change. Neither +command reads the local `agent.mcs.yml`, which is why the push comes first. Use this to route correctly — +name only the command you are recommending. A maker who is told which commands *not* to use has been handed +your reasoning instead of a next step. **If changes were applied:** diff --git a/tests/scripts/test_instruction_budget.py b/tests/scripts/test_instruction_budget.py index 620afb5d..94f6525b 100644 --- a/tests/scripts/test_instruction_budget.py +++ b/tests/scripts/test_instruction_budget.py @@ -308,6 +308,14 @@ def test_skill_states_changes_are_local_until_push(): step_10 = skill_text.split("## Step 10:")[1].split("## References")[0] assert "as deployed" in step_10 assert "does not exercise system instructions" in step_10 + # That explanation is routing logic for the model. A real run leaked it to + # the maker, who was told what /test is not for instead of a next step. + assert "Internal — do not say any of this to the maker." in step_10 + applied = step_10.split("**If changes were applied:**")[1].split("**If the maker declined")[0] + maker_facing = "\n".join( + ln for ln in applied.splitlines() if ln.lstrip().startswith(">") + ) + assert "/test" not in maker_facing def test_skill_requires_live_progress_tracking(): From 28be40b2766604d105d4ce13f30682b420dd6b2d Mon Sep 17 00:00:00 2001 From: Avery Milandin Date: Mon, 24 Aug 2026 19:44:05 -0700 Subject: [PATCH 06/10] fix(harden): report the contradiction check, and always name /push - The proposal summary reported length but said nothing about the contradiction re-check, so a silent pass looked the same as a skipped one. Step 8 now states the result next to the length, including when it is 'none', and names any surviving rule that had to be amended. - A run dropped /push entirely. 'Recommend, don't disqualify' was read as a limit on how many commands to name; it is not, and the applied path must name /push as its own instruction before anything about evaluation. Step 9 now states where the file landed and defers the instruction to Step 10, so the maker is told once rather than twice. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 96774230-f664-4887-baf1-a5bbfbd6cbb2 --- .../src/skills/instructions/harden/SKILL.md | 34 ++++++++++++++----- tests/scripts/test_instruction_budget.py | 19 ++++++++++- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md index 08eaedcb..61cc2c3b 100644 --- a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md +++ b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md @@ -46,9 +46,10 @@ and `workspace/`. They are not relative to this file. - **Speak the maker's language.** Never show `INSTR-*` ids, the filenames of this skill's own reference material, or the words "detector", "rule pack", or "probe". Describe findings in plain language. The maker's own instruction wording is *their* language and is shown verbatim. -- **Recommend, don't disqualify.** Name the command that fits and stop. Do not tell the maker which - commands are *not* right for their situation, or why — that is this skill's routing logic, not their - next step, and it reads as hedging. +- **Recommend, don't disqualify.** Do not tell the maker which commands are *not* right for their + situation, or why — that is this skill's routing logic, not their next step, and it reads as hedging. + This is not a limit on how many commands you name: when a path needs both `/push` and `/evaluate`, give + both, in order. - **TRACK PROGRESS**: Use the todo list tool to track your progress through this skill's steps. Create a todo list at the start with all the steps, mark each in-progress as you start it, and mark completed when done. Update it as you go rather than at the end — several steps here wait on the maker's reply, and a @@ -286,7 +287,18 @@ Present, in this order: For a contradiction with no obvious winner, present both directions as options and ask which behavior they intended, rather than choosing for them. -3. **The length**, in one line: the new total and the remaining headroom. +3. **The checks you ran on the proposal**, in one or two lines alongside each other: + + - **Contradictions**: state the result of the Step 6 re-check explicitly — that you read the proposed + text against the rules staying in place, and either that nothing conflicts or which surviving rule you + had to amend. Say this even when the answer is "none". The maker cannot see that this check happened, + and a silent pass is indistinguishable from a skipped one — which is the exact failure this skill + exists to catch, so leaving it implicit undermines the result. + - **Length**: the new total and the remaining headroom. + + > Checked the new wording against the rules that stay in place — no conflicts. Length: 3,140 of 8,000 + > characters, 4,860 to spare. + 4. **What this does not cover**: instructions do not fix a knowledge source the agent cannot retrieve. If the maker's reported examples looked like retrieval problems (see "What this checks"), say so here. @@ -328,11 +340,13 @@ Re-run the budget check without `--candidate` to confirm the written file measur `.local/harden/candidate.txt`. **Say where the change landed.** "I updated `agent.mcs.yml`" reads as done, and a maker who believes the -change is live will stop watching for the behavior they reported. Be explicit that this is a local file and -that the agent in Copilot Studio still has the old instructions: +change is live will stop watching for the behavior they reported. State that this is a local file: > I've updated your local copy of the agent. Your agent in Copilot Studio is still running the previous -> instructions — the change reaches it when you run `/push`. +> instructions. + +Leave it there — Step 10 gives the instruction to push. Do not write the two messages as separate +paragraphs saying the same thing. ## Step 10: Hand off to validation @@ -353,10 +367,14 @@ your reasoning instead of a next step. **If changes were applied:** +The applied path **must** name `/push`, as its own instruction, before anything about evaluation. This is +the only command that makes the change real, and it is the one most easily lost when it is bundled into a +paragraph about testing. Do not merge it into the `/evaluate` sentence, and do not leave it implied. + > Two things to know before you check whether this worked. > > The change is in your local copy only — your agent in Copilot Studio is still running the previous -> instructions. Run `/push` to send it. +> instructions. **Run `/push` to send it.** > > After that, `/evaluate` will turn the answers you didn't like into evaluation cases. It writes a CSV you > upload to the Copilot Studio Evaluation portal, which runs them against your agent and shows you whether diff --git a/tests/scripts/test_instruction_budget.py b/tests/scripts/test_instruction_budget.py index 94f6525b..294c5369 100644 --- a/tests/scripts/test_instruction_budget.py +++ b/tests/scripts/test_instruction_budget.py @@ -300,7 +300,9 @@ def test_skill_states_changes_are_local_until_push(): step_9 = skill_text.split("## Step 9:")[1].split("## Step 10:")[0] assert "still running the previous" in step_9 - assert "`/push`" in step_9 + # Step 9 states location only; Step 10 owns the instruction to push, so + # the maker is not told the same thing twice in adjacent paragraphs. + assert "Step 10 gives the instruction to push" in step_9 # /evaluate uploads to the Copilot Studio Evaluation portal and /test # drives a topic or workflow -- neither reads the local agent.mcs.yml, so @@ -316,6 +318,21 @@ def test_skill_states_changes_are_local_until_push(): ln for ln in applied.splitlines() if ln.lstrip().startswith(">") ) assert "/test" not in maker_facing + # /push is the only command that makes the change real, and a run dropped + # it once the routing rule was read as "name one command". + assert "must** name `/push`" in applied + assert "`/push`" in maker_facing + + +def test_skill_reports_the_contradiction_check_result(): + """A silent pass is indistinguishable from a skipped check, so the result + is stated next to the length rather than left implied.""" + skill = _SKILL_ROOT / "src" / "skills" / "instructions" / "harden" / "SKILL.md" + step_8 = (skill.read_text(encoding="utf-8") + .split("## Step 8:")[1].split("## Step 9:")[0]) + + assert "state the result of the Step 6 re-check explicitly" in step_8 + assert 'Say this even when the answer is "none"' in step_8 def test_skill_requires_live_progress_tracking(): From 58834c93b9314ffaffe8d03c7ee3434359771f5d Mon Sep 17 00:00:00 2001 From: Avery Milandin Date: Tue, 25 Aug 2026 13:13:38 -0700 Subject: [PATCH 07/10] fix(harden): correct how /evaluate delivers test sets The harden handoff told makers that /evaluate writes a CSV they upload to the Copilot Studio Evaluation portal. It does not: evaluations/create writes .mcs.yml files to {agent.folder}/evaluations/ and pushes them to Copilot Studio as botcomponent records, which the maker runs from the Evaluation tab. I took the CSV wording from evaluate.prompt.md, which contradicts the skill it routes to. Its 'workspace/tests/{date}/' path is referenced nowhere else in the repo and no such directory exists, so that line is corrected here too rather than left to mislead the next reader. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 96774230-f664-4887-baf1-a5bbfbd6cbb2 --- .../.github/prompts/evaluate.prompt.md | 2 +- .../src/skills/instructions/harden/SKILL.md | 21 ++++++++++--------- tests/scripts/test_instruction_budget.py | 19 +++++++++++------ 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/solutions/ess-maker-skills/.github/prompts/evaluate.prompt.md b/solutions/ess-maker-skills/.github/prompts/evaluate.prompt.md index 0b799bf4..1f3003b3 100644 --- a/solutions/ess-maker-skills/.github/prompts/evaluate.prompt.md +++ b/solutions/ess-maker-skills/.github/prompts/evaluate.prompt.md @@ -21,4 +21,4 @@ and STOP. Otherwise proceed. - **delete** -> read `src/skills/evaluations/delete/SKILL.md` and follow it 4. If the answer is ambiguous, ask once more before routing. -Test sets generated by /evaluate land in `workspace/tests/{date}/` as CSV files ready to upload to the Copilot Studio Evaluation portal. +Test sets generated by /evaluate are written to `{agent.folder}/evaluations/` as `.mcs.yml` files and pushed to Copilot Studio as `botcomponent` records, ready to run from the Evaluation tab there. diff --git a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md index 61cc2c3b..d04ad6f2 100644 --- a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md +++ b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md @@ -357,13 +357,14 @@ findings. Instruction changes are behavioral changes, and this skill has no way to demonstrate that the new text produces better answers. Say that plainly and route the maker onward. -**Internal — do not say any of this to the maker.** `/evaluate` *authors* evaluation cases; the CSVs land -in `workspace/tests/{date}/` and are uploaded to the Copilot Studio Evaluation portal, which runs them -against the agent **as deployed**. `/test` drives a **topic or a workflow** to debug its runtime behavior; -it does not exercise system instructions, so it is not the way to check an instruction change. Neither -command reads the local `agent.mcs.yml`, which is why the push comes first. Use this to route correctly — -name only the command you are recommending. A maker who is told which commands *not* to use has been handed -your reasoning instead of a next step. +**Internal — do not say any of this to the maker.** `/evaluate` *authors* evaluation cases: it writes them +to `{agent.folder}/evaluations/` as `.mcs.yml` and pushes them to Copilot Studio as `botcomponent` records, +which the maker then runs from the Evaluation tab. The run happens against the agent **as deployed**. +`/test` drives a **topic or a workflow** to debug its runtime behavior; it does not exercise system +instructions, so it is not the way to check an instruction change. Neither command reads the local +`agent.mcs.yml`, which is why the push comes first. Use this to route correctly — name only the command you +are recommending. A maker who is told which commands *not* to use has been handed your reasoning instead of +a next step. **If changes were applied:** @@ -376,9 +377,9 @@ paragraph about testing. Do not merge it into the `/evaluate` sentence, and do n > The change is in your local copy only — your agent in Copilot Studio is still running the previous > instructions. **Run `/push` to send it.** > -> After that, `/evaluate` will turn the answers you didn't like into evaluation cases. It writes a CSV you -> upload to the Copilot Studio Evaluation portal, which runs them against your agent and shows you whether -> the behaviour actually changed. +> After that, `/evaluate` will turn the answers you didn't like into evaluation cases and add them to your +> agent in Copilot Studio. You run them from the Evaluation tab there, and the results show you whether the +> behaviour actually changed. Where the maker gave specific bad responses in Step 2, carry them forward: those are the highest-value evaluation rows available, and they are the only direct evidence of whether this pass worked. Offer to run diff --git a/tests/scripts/test_instruction_budget.py b/tests/scripts/test_instruction_budget.py index 294c5369..695f7c65 100644 --- a/tests/scripts/test_instruction_budget.py +++ b/tests/scripts/test_instruction_budget.py @@ -304,15 +304,22 @@ def test_skill_states_changes_are_local_until_push(): # the maker is not told the same thing twice in adjacent paragraphs. assert "Step 10 gives the instruction to push" in step_9 - # /evaluate uploads to the Copilot Studio Evaluation portal and /test - # drives a topic or workflow -- neither reads the local agent.mcs.yml, so - # the push has to come first or the evaluation measures the old text. + # /evaluate pushes eval sets to Copilot Studio as botcomponent records and + # the maker runs them from the Evaluation tab; /test drives a topic or + # workflow. Neither reads the local agent.mcs.yml, so the push has to come + # first or the evaluation measures the old text. step_10 = skill_text.split("## Step 10:")[1].split("## References")[0] - assert "as deployed" in step_10 - assert "does not exercise system instructions" in step_10 + # Markdown wraps at 110 columns, so a phrase can span a newline. + flat_10 = " ".join(step_10.split()) + assert "as deployed" in flat_10 + assert "does not exercise system instructions" in flat_10 + # It does not write CSVs to workspace/tests/, a path that exists nowhere + # else in the repo. + assert "workspace/tests" not in flat_10 + assert "{agent.folder}/evaluations/" in flat_10 # That explanation is routing logic for the model. A real run leaked it to # the maker, who was told what /test is not for instead of a next step. - assert "Internal — do not say any of this to the maker." in step_10 + assert "Internal — do not say any of this to the maker." in flat_10 applied = step_10.split("**If changes were applied:**")[1].split("**If the maker declined")[0] maker_facing = "\n".join( ln for ln in applied.splitlines() if ln.lstrip().startswith(">") From ea8a7f04e95fe88823fc4cccbd411b05304ace11 Mon Sep 17 00:00:00 2001 From: Avery Milandin Date: Tue, 25 Aug 2026 15:50:39 -0700 Subject: [PATCH 08/10] fix: send test-pane turns from the focused element, not a stale locator _drive_turn located the message box via a placeholder-based selector and then called box.press("Enter"). Playwright re-resolves a locator on every call, and the test pane drops the placeholder attribute once the box has text -- so the press could not re-resolve the box it had just filled and timed out after 30s. The prompt was left typed but unsent, surfacing as a hang rather than a send failure. Press Enter via the page keyboard so the just-filled, still-focused element receives it, and prefer the stable data-testid selector for the input over the placeholder-based one. Verified end-to-end against a live Copilot Studio test pane: drive_topic.py now completes a turn and captures the reply instead of timing out. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 96774230-f664-4887-baf1-a5bbfbd6cbb2 --- solutions/ess-maker-skills/scripts/cdp_driver.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/solutions/ess-maker-skills/scripts/cdp_driver.py b/solutions/ess-maker-skills/scripts/cdp_driver.py index 8a1705c8..a919c281 100644 --- a/solutions/ess-maker-skills/scripts/cdp_driver.py +++ b/solutions/ess-maker-skills/scripts/cdp_driver.py @@ -46,6 +46,7 @@ # Candidate selectors for the test-pane message input (first visible wins). INPUT_CANDIDATES = [ + 'textarea[data-testid="send box text area"]', 'textarea[placeholder*="Ask" i]', 'textarea[aria-label*="message" i]', '[contenteditable="true"][aria-label*="message" i]', @@ -399,7 +400,10 @@ def _on_done(params): box.click() box.fill(text) - box.press("Enter") + # Send from the focused element rather than re-resolving the locator: a + # placeholder-based selector stops matching once the box has text, so + # box.press() would time out on a box that was just filled successfully. + page.keyboard.press("Enter") if _DEBUG: print(f" [drive] {text!r}", file=sys.stderr) From 90061e8cfa743f3369c095d7bf6d6572bfa2daa4 Mon Sep 17 00:00:00 2001 From: Avery Milandin Date: Tue, 25 Aug 2026 16:18:25 -0700 Subject: [PATCH 09/10] fix: route instruction validation to /test, not /evaluate Step 10 described /push, /test, and /evaluate incorrectly. /push writes agent.mcs.yml to Copilot Studio through Dataverse and does not publish, so the change reaches the draft the test pane answers from while published channels keep serving the previous instructions. Step 10 now says that, so a maker knows the push alone does not reach their users. /test drives the test pane and captures the reply, which does exercise system instructions -- verified against a live agent, where an instruction pushed without a publish changed the test-pane answers. Step 10 previously claimed the opposite and sent the maker to /evaluate instead. /evaluate authors evaluation cases and is not the shortest path from an instruction change to an observed reply, so it is no longer named here. Also records that a pushed change can be served from a cached definition for several minutes, so stale behaviour is not read as a failed fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 96774230-f664-4887-baf1-a5bbfbd6cbb2 --- .../src/skills/instructions/harden/SKILL.md | 48 ++++++++++--------- tests/scripts/test_instruction_budget.py | 20 ++++---- 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md index d04ad6f2..65840c75 100644 --- a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md +++ b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md @@ -357,52 +357,54 @@ findings. Instruction changes are behavioral changes, and this skill has no way to demonstrate that the new text produces better answers. Say that plainly and route the maker onward. -**Internal — do not say any of this to the maker.** `/evaluate` *authors* evaluation cases: it writes them -to `{agent.folder}/evaluations/` as `.mcs.yml` and pushes them to Copilot Studio as `botcomponent` records, -which the maker then runs from the Evaluation tab. The run happens against the agent **as deployed**. -`/test` drives a **topic or a workflow** to debug its runtime behavior; it does not exercise system -instructions, so it is not the way to check an instruction change. Neither command reads the local -`agent.mcs.yml`, which is why the push comes first. Use this to route correctly — name only the command you -are recommending. A maker who is told which commands *not* to use has been handed your reasoning instead of -a next step. +**Internal — do not say any of this to the maker.** `/push` writes the local `agent.mcs.yml` to Copilot +Studio through Dataverse. It does **not** publish the agent: the change lands in the agent's draft, which is +what the Copilot Studio test pane answers from, while published channels keep serving the previous +instructions until the maker publishes. `/test` drives that test pane — it sends a prompt and captures the +reply — so it is the command that shows whether the new instructions changed the answers. A pushed +instruction change is not always visible immediately; the runtime can serve a cached definition for several +minutes. Neither command reads the local `agent.mcs.yml`, which is why the push comes first. Name only the +command you are recommending. A maker who is told which commands *not* to use has been handed your +reasoning instead of a next step. **If changes were applied:** -The applied path **must** name `/push`, as its own instruction, before anything about evaluation. This is +The applied path **must** name `/push`, as its own instruction, before anything about testing. This is the only command that makes the change real, and it is the one most easily lost when it is bundled into a -paragraph about testing. Do not merge it into the `/evaluate` sentence, and do not leave it implied. +paragraph about testing. Do not merge it into the `/test` sentence, and do not leave it implied. > Two things to know before you check whether this worked. > > The change is in your local copy only — your agent in Copilot Studio is still running the previous -> instructions. **Run `/push` to send it.** +> instructions. **Run `/push` to send it.** That updates the agent in Copilot Studio but does not publish +> it, so anyone using the published agent keeps the previous instructions until you publish. > -> After that, `/evaluate` will turn the answers you didn't like into evaluation cases and add them to your -> agent in Copilot Studio. You run them from the Evaluation tab there, and the results show you whether the -> behaviour actually changed. +> Then run `/test` and ask the questions that produced the answers you didn't like. It drives your agent +> and shows you the actual replies, which is the only way to see whether the new instructions changed the +> behaviour. A pushed change can take a few minutes to reach the test pane — if you still see the old +> behaviour, wait and ask again before concluding it didn't work. Where the maker gave specific bad responses in Step 2, carry them forward: those are the highest-value -evaluation rows available, and they are the only direct evidence of whether this pass worked. Offer to run -`/evaluate` with them. +probes available, and they are the only direct evidence of whether this pass worked. Offer to run `/test` +with them. Also mention, once, that a change intended to prevent a bad answer can also cause the agent to decline good -questions — so the evaluation set should include a few normal, in-scope questions alongside the failing -ones. Without those rows, an agent that has started refusing everything still scores clean. +questions — so the questions asked should include a few normal, in-scope ones alongside the failing ones. +Without those, an agent that has started refusing everything still looks fixed. **If the maker declined or deferred the proposal:** > Nothing has changed, locally or in Copilot Studio. If you want to find out whether the behaviour I -> described actually shows up, `/evaluate` will build an evaluation set you can run against the agent as -> it stands — that gives you a baseline before changing anything. +> described actually shows up, `/test` drives your agent so you can ask the questions directly — that +> gives you a baseline before changing anything. **If nothing was proposed:** > I didn't find anything worth changing in the instructions. That doesn't mean the agent answers well — > instructions are only one input, and knowledge sources and topics matter at least as much. > -> `/evaluate` will build an evaluation set to run against the agent, which finds behaviour problems this -> review cannot see. If you suspect one specific topic or workflow is misbehaving, `/test` drives that -> component directly. +> `/test` drives your agent so you can ask the questions that concern you and see the actual replies. That +> finds behaviour problems this review cannot see. Reading the instructions cannot tell you what the agent actually says. Do not let a clean review stand as evidence that the agent is fine. diff --git a/tests/scripts/test_instruction_budget.py b/tests/scripts/test_instruction_budget.py index 695f7c65..b6844d9e 100644 --- a/tests/scripts/test_instruction_budget.py +++ b/tests/scripts/test_instruction_budget.py @@ -304,19 +304,21 @@ def test_skill_states_changes_are_local_until_push(): # the maker is not told the same thing twice in adjacent paragraphs. assert "Step 10 gives the instruction to push" in step_9 - # /evaluate pushes eval sets to Copilot Studio as botcomponent records and - # the maker runs them from the Evaluation tab; /test drives a topic or - # workflow. Neither reads the local agent.mcs.yml, so the push has to come - # first or the evaluation measures the old text. + # /push writes agent.mcs.yml to Copilot Studio without publishing, so the + # change reaches the draft the test pane answers from. /test drives that + # pane and captures the reply, which is how an instruction change is + # checked. Neither reads the local agent.mcs.yml, so the push comes first. step_10 = skill_text.split("## Step 10:")[1].split("## References")[0] # Markdown wraps at 110 columns, so a phrase can span a newline. flat_10 = " ".join(step_10.split()) - assert "as deployed" in flat_10 - assert "does not exercise system instructions" in flat_10 + assert "does not publish" in flat_10 + assert "drives that test pane" in flat_10 + # A pushed change can be served from a cached definition for minutes; a + # maker who is not told that reads stale behaviour as a failed fix. + assert "cached definition" in flat_10 # It does not write CSVs to workspace/tests/, a path that exists nowhere # else in the repo. assert "workspace/tests" not in flat_10 - assert "{agent.folder}/evaluations/" in flat_10 # That explanation is routing logic for the model. A real run leaked it to # the maker, who was told what /test is not for instead of a next step. assert "Internal — do not say any of this to the maker." in flat_10 @@ -324,7 +326,9 @@ def test_skill_states_changes_are_local_until_push(): maker_facing = "\n".join( ln for ln in applied.splitlines() if ln.lstrip().startswith(">") ) - assert "/test" not in maker_facing + # /test is what actually exercises the new instructions, so the applied + # path has to name it rather than leaving the maker without a way to look. + assert "/test" in maker_facing # /push is the only command that makes the change real, and a run dropped # it once the routing rule was read as "name one command". assert "must** name `/push`" in applied From a77254938df345528d6ecdfb7b5768707de75a46 Mon Sep 17 00:00:00 2001 From: Avery Milandin Date: Tue, 25 Aug 2026 16:25:44 -0700 Subject: [PATCH 10/10] fix: stop gating harden intake on a concrete example Step 2 told the run it did not have an answer yet whenever the maker named a category rather than a behavior, and not to proceed until it got one. A maker reporting a general concern about responses was asked for a verbatim question/answer pair repeatedly, which reads as a hard requirement for a capability they had just asked for. It also contradicted Step 6, which already has a branch for a theme named without an example and a branch for no reported problem at all. The run had somewhere to go the whole time. Step 2 now follows up once for specificity, then proceeds on whatever the second answer gives it and says what it is working from. The prohibition on offering menus is unchanged -- a menu invites a category, whereas an open question sometimes gets a behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 96774230-f664-4887-baf1-a5bbfbd6cbb2 --- .../src/skills/instructions/harden/SKILL.md | 15 ++++++++---- tests/scripts/test_instruction_budget.py | 23 +++++++++++++++++-- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md index 65840c75..7008dd18 100644 --- a/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md +++ b/solutions/ess-maker-skills/src/skills/instructions/harden/SKILL.md @@ -116,11 +116,16 @@ instead, and a category tells you nothing you can anchor a change to. Ask this a > If nothing specific has gone wrong, that's fine too — say so and I'll check the instructions for > contradictions and gaps and tell you what I find. -**If the answer names a category rather than a behavior, you do not have an answer yet — ask again.** -"General concerns", "the usual problems", "hallucination", or picking one of your own examples back are -labels, not evidence. Follow up in prose — *"What has it been doing that concerns you?"* — and wait. Do -not proceed to Step 3 on a label. A proposal built from a category is a proposal built from nothing, and -it will read as generic hardening because that is what it is. +**If the answer names a category rather than a behavior, follow up once — then proceed with whatever the +second answer gives you.** "General concerns", "the usual problems", "hallucination", or picking one of +your own examples back are labels rather than evidence, and one prose follow-up — *"What has it been doing +that concerns you?"* — often turns a label into a behavior class worth scoping to. + +**Examples are never required.** Ask once, accept the answer, and move on. A maker who cannot produce a +verbatim exchange still has a real concern, and a second or third request for one reads as a gate on a +capability they asked for. If the follow-up produces nothing more specific, that is itself an answer: you +have a theme or no reported problem, and Step 6 has a branch for each. Say what you are working from — +*"I'll work from that as a general concern and tell you what I find"* — and go to Step 3. Record their answer. Do not paraphrase a vague answer into a specific complaint — if they said "it makes things up sometimes" without an example, you have a **theme**, not a case, and Step 6 treats those diff --git a/tests/scripts/test_instruction_budget.py b/tests/scripts/test_instruction_budget.py index b6844d9e..f01f3c3b 100644 --- a/tests/scripts/test_instruction_budget.py +++ b/tests/scripts/test_instruction_budget.py @@ -362,6 +362,25 @@ def test_skill_forbids_option_menus_at_intake(): skill = _SKILL_ROOT / "src" / "skills" / "instructions" / "harden" / "SKILL.md" step_2 = (skill.read_text(encoding="utf-8") .split("## Step 2:")[1].split("## Step 3:")[0]) + flat_2 = " ".join(step_2.split()) - assert "Do not offer numbered options" in step_2 - assert "ask again" in step_2 + assert "Do not offer numbered options" in flat_2 + # One prose follow-up turns a label into something scopeable often enough + # to be worth asking for. + assert "follow up once" in flat_2 + + +def test_skill_does_not_gate_intake_on_concrete_examples(): + """A run asked repeatedly for a verbatim question/answer pair before it + would look at anything, which reads as a gate on the capability. Step 6 + has branches for a theme and for no reported problem, so the run has + somewhere to go without an example.""" + skill = _SKILL_ROOT / "src" / "skills" / "instructions" / "harden" / "SKILL.md" + step_2 = (skill.read_text(encoding="utf-8") + .split("## Step 2:")[1].split("## Step 3:")[0]) + flat_2 = " ".join(step_2.split()) + + assert "Examples are never required" in flat_2 + # The earlier wording stalled the run on a label instead of routing it. + assert "Do not proceed to Step 3" not in flat_2 + assert "you do not have an answer yet" not in flat_2