From d39ef472ab96436c5bc9351dae7f24ea1f4165dc Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 26 Jul 2026 01:02:29 +0200 Subject: [PATCH 1/6] docs(agents): name agent-skills as the owner of bundled skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Agent definition locations section listed `plugins/agentic-engineering/skills/agent-improvement/SKILL.md` as a version-controlled authoring surface. It is not one: that file carries `metadata.github-repo: …/agent-skills` (tag v1.9.1) and the update-agent-skills workflow re-pulls it, so an edit there is silently reverted — no conflict, no CI failure, no signal. This is not hypothetical. The next generic fix queued for that skill is a change to its step-5 verdicts, and following the contract as written would have routed it straight into the discarded copy. Its neighbours in the same bullet were correct — `agents/agent-improver.agent.md` carries no provenance and genuinely is authored in agent-plugins — so the bullet now splits by owning repository and states the per-FILE test, since one plugin directory holds both kinds. Guarded three ways, all RED-proven: the owner sentence, the explicit "not an authoring surface" marking, and a check that the bundled SKILL.md still carries sync provenance at all (so the contract text stops being trusted if the sync arrangement ever changes). Co-Authored-By: Claude Opus 5 (1M context) --- .../agent-role-delivery-contract.test.sh | 18 ++++++++++++++++ AGENTS.md | 21 ++++++++++++++----- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/.claude/scripts/agent-role-delivery-contract.test.sh b/.claude/scripts/agent-role-delivery-contract.test.sh index 3b459cd0..db7f874c 100755 --- a/.claude/scripts/agent-role-delivery-contract.test.sh +++ b/.claude/scripts/agent-role-delivery-contract.test.sh @@ -50,6 +50,24 @@ grep -Fq '### Authority model' "${constitution}" || grep -Fq 'plugins/agentic-engineering/agents/agent-improver.agent.md' "${constitution}" || fail "consumer does not name the upstream Agent Improver source" +# The bundled SKILL.md is SYNCED from devantler-tech/agent-skills (it carries +# metadata.github-repo and the update-agent-skills workflow re-pulls it), so an edit there +# is silently reverted. The consumer listed it as an authoring surface until 2026-07-25, +# which would route a generic fix into a file that discards it. Assert the correct owner is +# named, and that the synced path is not presented as the place to change behaviour. +assert_prose "\`devantler-tech/agent-skills\`** authors the bundled" \ + "consumer does not name agent-skills as the owner of bundled skills" +assert_prose "not** an authoring surface" \ + "consumer does not mark the synced SKILL.md copy as a non-authoring surface" + +# Provenance is a per-FILE question: the same plugin directory holds synced skills and +# locally-authored agents, so a per-directory rule is wrong in one direction or the other. +bundled_skill="${repo_root}/libraries/agent-plugins/plugins/agentic-engineering/skills/agent-improvement/SKILL.md" +if [ -f "${bundled_skill}" ]; then + grep -q 'github-repo' "${bundled_skill}" || + fail "bundled agent-improvement/SKILL.md no longer carries sync provenance — re-check whether it is still synced before trusting the contract text" +fi + # Every machine-readable entrypoint pointer must resolve to an agent the pinned plugin # actually BUNDLES. Derived from the submodule rather than hard-coded, so the next upstream # rename cannot leave this consumer pointing at a file that no longer exists — which is diff --git a/AGENTS.md b/AGENTS.md index 9c668012..b052b1d4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -263,11 +263,22 @@ definition surface, and an installed/cache copy is never an authoring target. script, the provider-neutral desired state, plugin settings, and the Cursor loader source. The local Agent Improver agent/skill forks are retired, and so is the standalone FinOps agent fork — the reviewed plugin is the source for both roles. -- The generic upstream source in `devantler-tech/agent-plugins`, specifically - `plugins/agentic-engineering/agents/agent-improver.agent.md`, - `plugins/agentic-engineering/skills/agent-improvement/SKILL.md`, the plugin README/desired state, - and their manifest/contract validation. Change generic behaviour there first, merge it, then update - this consumer's `libraries/agent-plugins` gitlink and copied desired state. +- The generic upstream source, which is split across **two** repositories. **Check the file's own + provenance before editing it — the question is per-FILE, never per-directory**, because one plugin + directory mixes both kinds: + - **`devantler-tech/agent-plugins`** authors + `plugins/agentic-engineering/agents/agent-improver.agent.md`, the plugin README/desired state, and + their manifest/contract validation. These carry **no** `metadata.github-repo`. + - **`devantler-tech/agent-skills`** authors the bundled *skills*, including `agent-improvement/`. + 🔴 The copy at `plugins/agentic-engineering/skills/agent-improvement/SKILL.md` carries + `metadata.github-repo: …/agent-skills` and is re-pulled by the `update-agent-skills` workflow, so + editing it there is **silently reverted** — no conflict, no CI failure, no signal. It is a synced + artifact, **not** an authoring surface. + + Verify with `grep -l "github-repo" plugins/*/skills/*/SKILL.md` (synced) versus + `grep -L "github-repo" plugins/*/agents/*.agent.md` (authored here). Change generic behaviour in the + **owning** repository first, merge it, then update this consumer's `libraries/agent-plugins` gitlink + and copied desired state. **Runtime-local surfaces — back up before editing, verify in place, and record before/after in native memory and the run report:** From c492ed3672efdf47d984ff1a30f4732d806dbfff Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 26 Jul 2026 01:42:33 +0200 Subject: [PATCH 2/6] fix(agents): validate the provenance value, not just the key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings, all valid. The provenance guard matched a bare `github-repo`, which only proves the file is synced from somewhere. Had the upstream moved to a different repository — the one case where this contract's routing text goes wrong and the guard is the only thing that would notice — it would have stayed green. Both the guard and the documented command now match the key together with its `devantler-tech/agent-skills` value. The documented verification globs started at `plugins/...`, but from the consumer root the bundled files live under `libraries/agent-plugins/plugins/...`, so they matched nothing. A reader following them got an empty result and could reasonably conclude no file is synced. Both are now repo-relative and verified to return results (11 synced skills, 5 locally-authored agents). The prose assertions matched generic fragments that unrelated ownership text could satisfy, so the specific routing rule could have been deleted with the guard still green. Both now name the bundled skill path. Both sharpened guards are RED-proven: repointing the upstream value fails the provenance check, and removing the path from the contract fails the prose check. Co-Authored-By: Claude Opus 5 (1M context) --- .../agent-role-delivery-contract.test.sh | 22 ++++++++++++---- AGENTS.md | 25 +++++++++++++------ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/.claude/scripts/agent-role-delivery-contract.test.sh b/.claude/scripts/agent-role-delivery-contract.test.sh index db7f874c..55353b46 100755 --- a/.claude/scripts/agent-role-delivery-contract.test.sh +++ b/.claude/scripts/agent-role-delivery-contract.test.sh @@ -53,19 +53,31 @@ grep -Fq 'plugins/agentic-engineering/agents/agent-improver.agent.md' "${constit # The bundled SKILL.md is SYNCED from devantler-tech/agent-skills (it carries # metadata.github-repo and the update-agent-skills workflow re-pulls it), so an edit there # is silently reverted. The consumer listed it as an authoring surface until 2026-07-25, -# which would route a generic fix into a file that discards it. Assert the correct owner is -# named, and that the synced path is not presented as the place to change behaviour. +# which would route a generic fix into a file that discards it. +# +# Both assertions name the bundled skill PATH. Generic fragments ("authors the bundled", +# "not an authoring surface") could otherwise be satisfied by unrelated ownership prose +# elsewhere in the contract, which would let the specific routing rule be deleted while the +# guard stayed green. +skill_path='plugins/agentic-engineering/skills/agent-improvement/SKILL.md' assert_prose "\`devantler-tech/agent-skills\`** authors the bundled" \ "consumer does not name agent-skills as the owner of bundled skills" -assert_prose "not** an authoring surface" \ +assert_prose "${skill_path}\` carries" \ + "consumer does not name the bundled agent-improvement/SKILL.md as the synced copy" +assert_prose "It is a synced artifact, **not** an authoring surface" \ "consumer does not mark the synced SKILL.md copy as a non-authoring surface" # Provenance is a per-FILE question: the same plugin directory holds synced skills and # locally-authored agents, so a per-directory rule is wrong in one direction or the other. +# +# Match the github-repo KEY TOGETHER WITH ITS VALUE. A bare `grep -q github-repo` proves only +# that the file is synced from SOMEWHERE — it would stay green if the upstream moved to a +# different repository, which is exactly the case where the contract's routing text becomes +# wrong and this guard is the only thing that would notice. bundled_skill="${repo_root}/libraries/agent-plugins/plugins/agentic-engineering/skills/agent-improvement/SKILL.md" if [ -f "${bundled_skill}" ]; then - grep -q 'github-repo' "${bundled_skill}" || - fail "bundled agent-improvement/SKILL.md no longer carries sync provenance — re-check whether it is still synced before trusting the contract text" + grep -q 'github-repo: https://github.com/devantler-tech/agent-skills' "${bundled_skill}" || + fail "bundled agent-improvement/SKILL.md no longer declares devantler-tech/agent-skills as its upstream — re-check the owning repository before trusting the contract text" fi # Every machine-readable entrypoint pointer must resolve to an agent the pinned plugin diff --git a/AGENTS.md b/AGENTS.md index b052b1d4..8c766096 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -271,14 +271,23 @@ definition surface, and an installed/cache copy is never an authoring target. their manifest/contract validation. These carry **no** `metadata.github-repo`. - **`devantler-tech/agent-skills`** authors the bundled *skills*, including `agent-improvement/`. 🔴 The copy at `plugins/agentic-engineering/skills/agent-improvement/SKILL.md` carries - `metadata.github-repo: …/agent-skills` and is re-pulled by the `update-agent-skills` workflow, so - editing it there is **silently reverted** — no conflict, no CI failure, no signal. It is a synced - artifact, **not** an authoring surface. - - Verify with `grep -l "github-repo" plugins/*/skills/*/SKILL.md` (synced) versus - `grep -L "github-repo" plugins/*/agents/*.agent.md` (authored here). Change generic behaviour in the - **owning** repository first, merge it, then update this consumer's `libraries/agent-plugins` gitlink - and copied desired state. + `metadata.github-repo: https://github.com/devantler-tech/agent-skills` and is re-pulled by the + `update-agent-skills` workflow, so editing it there is **silently reverted** — no conflict, no CI + failure, no signal. It is a synced artifact, **not** an authoring surface. + + Verify **from the monorepo root**, matching the key *and its value* — a bare `github-repo` test only + proves the file is synced from *somewhere*, not from the repo you are about to open a PR against: + + ```sh + # synced — edit these upstream, in the repo the value names: + grep -l 'github-repo: https://github.com/devantler-tech/agent-skills' \ + libraries/agent-plugins/plugins/*/skills/*/SKILL.md + # authored in agent-plugins — safe to edit there: + grep -L 'github-repo' libraries/agent-plugins/plugins/*/agents/*.agent.md + ``` + + Change generic behaviour in the **owning** repository first, merge it, then update this consumer's + `libraries/agent-plugins` gitlink and copied desired state. **Runtime-local surfaces — back up before editing, verify in place, and record before/after in native memory and the run report:** From a941da92be790c119764bb7f3d6d20fde3d4bdc7 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 26 Jul 2026 01:50:31 +0200 Subject: [PATCH 3/6] fix(agents): anchor the provenance match and fail closed on a missing skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three P2 findings, all valid. The provenance guard was still a substring match, so an upstream renamed with the same prefix — `agent-skills-v2` — passed it, missing exactly the ownership move the guard claims to detect. Verified against a probe before fixing. The match is now anchored to the whole metadata value. The check also sat behind `[ -f ]`, so a pinned plugin revision that renamed or dropped the skill would skip the assertion entirely and leave the contract green while AGENTS.md routed edits through a path that no longer exists. It now fails closed, as the test already did for the plugin agent directory. The documented rollout skipped a hop. For a skill owned by agent-skills, merging there does not make the change available here: `update-agent-skills` must re-pull it into agent-plugins and that generated PR must merge before this repo's gitlink can expose it. Following the old sequence would pin a revision still carrying the old skill and read as a finished rollout. The two owners now have separate, explicit sequences. Both guards RED-proven: the `agent-skills-v2` probe and a removed skill each fail with their own message. Co-Authored-By: Claude Opus 5 (1M context) --- .../scripts/agent-role-delivery-contract.test.sh | 15 +++++++++++---- AGENTS.md | 14 ++++++++++++-- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/.claude/scripts/agent-role-delivery-contract.test.sh b/.claude/scripts/agent-role-delivery-contract.test.sh index 55353b46..69662acc 100755 --- a/.claude/scripts/agent-role-delivery-contract.test.sh +++ b/.claude/scripts/agent-role-delivery-contract.test.sh @@ -74,11 +74,18 @@ assert_prose "It is a synced artifact, **not** an authoring surface" \ # that the file is synced from SOMEWHERE — it would stay green if the upstream moved to a # different repository, which is exactly the case where the contract's routing text becomes # wrong and this guard is the only thing that would notice. +# FAIL CLOSED on a missing file. A `[ -f ] &&` conditional would silently skip the whole +# assertion if the pinned plugin revision renamed or dropped the skill — leaving AGENTS.md +# routing edits through a path that no longer exists, with the contract test still green. +# +# ANCHOR THE VALUE'S END. An unanchored substring accepts any longer name with the same +# prefix (`agent-skills-v2`), so it would miss exactly the ownership move it claims to +# detect. Verified: the probe `…/agent-skills-v2` passes the unanchored form. bundled_skill="${repo_root}/libraries/agent-plugins/plugins/agentic-engineering/skills/agent-improvement/SKILL.md" -if [ -f "${bundled_skill}" ]; then - grep -q 'github-repo: https://github.com/devantler-tech/agent-skills' "${bundled_skill}" || - fail "bundled agent-improvement/SKILL.md no longer declares devantler-tech/agent-skills as its upstream — re-check the owning repository before trusting the contract text" -fi +[ -f "${bundled_skill}" ] || + fail "bundled agent-improvement/SKILL.md is missing at the pinned plugin revision — AGENTS.md routes generic skill edits through this path, so its absence invalidates the contract text" +grep -qE '^[[:space:]]*github-repo:[[:space:]]*https://github\.com/devantler-tech/agent-skills[[:space:]]*$' "${bundled_skill}" || + fail "bundled agent-improvement/SKILL.md no longer declares exactly devantler-tech/agent-skills as its upstream — re-check the owning repository before trusting the contract text" # Every machine-readable entrypoint pointer must resolve to an agent the pinned plugin # actually BUNDLES. Derived from the submodule rather than hard-coded, so the next upstream diff --git a/AGENTS.md b/AGENTS.md index 8c766096..b53e714c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -286,8 +286,18 @@ definition surface, and an installed/cache copy is never an authoring target. grep -L 'github-repo' libraries/agent-plugins/plugins/*/agents/*.agent.md ``` - Change generic behaviour in the **owning** repository first, merge it, then update this consumer's - `libraries/agent-plugins` gitlink and copied desired state. + Change generic behaviour in the **owning** repository first. The rollout then differs by owner, and + **the skills path has an extra hop that is easy to skip**: + - *Authored in `agent-plugins`* (agents, README, desired state): merge there, then bump this + consumer's `libraries/agent-plugins` gitlink. + - *Authored in `agent-skills`* (bundled skills): merge there, **then wait for `update-agent-skills` + to re-pull it into `agent-plugins` and for THAT generated PR to merge**, and only then bump the + gitlink. Bumping straight after the `agent-skills` merge pins a revision that still carries the + **old** skill — the change is real upstream and absent here, which reads as a completed rollout + while nothing has actually shipped to this deployment. Confirm by reading the skill's content at + the pinned revision, never by the upstream PR being merged. + + Finally, update the copied desired state. **Runtime-local surfaces — back up before editing, verify in place, and record before/after in native memory and the run report:** From e86e1f0b1f05e9a1ac66e1818ca4792e0e7feba4 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 26 Jul 2026 01:59:01 +0200 Subject: [PATCH 4/6] fix(agents): scope the contract guards to the paragraph and the frontmatter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three P2 findings, all valid, all three scope holes. The prose assertions searched the whole flattened contract, so the specific rule could have been deleted from the agent-improvement paragraph while a generic sentence about some other synced artifact kept the guard green. Path, provenance value and non-authoring rule are now asserted within one extracted paragraph. The provenance grep scanned the entire skill file, so a documentation example or code block in the body would satisfy it after the real metadata field was removed or repointed — the guard would claim to verify provenance that was gone. It now matches inside the YAML frontmatter only. An uninitialised `libraries/agent-plugins` reported "the skill is missing at the pinned revision", which reads as upstream contract drift and hides the actionable fix. A fresh checkout is a normal local state, so it is now detected first and points at the repository's own init helper, matching how the entrypoint check already behaves. RED-proven with the exact scenarios: provenance present in the body but absent from the frontmatter, the non-authoring sentence relocated to another paragraph, and the plugin directory removed. Each fails with its own message; all three passed before this change. Co-Authored-By: Claude Opus 5 (1M context) --- .../agent-role-delivery-contract.test.sh | 45 ++++++++++++++++--- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/.claude/scripts/agent-role-delivery-contract.test.sh b/.claude/scripts/agent-role-delivery-contract.test.sh index 69662acc..291a7f37 100755 --- a/.claude/scripts/agent-role-delivery-contract.test.sh +++ b/.claude/scripts/agent-role-delivery-contract.test.sh @@ -62,10 +62,30 @@ grep -Fq 'plugins/agentic-engineering/agents/agent-improver.agent.md' "${constit skill_path='plugins/agentic-engineering/skills/agent-improvement/SKILL.md' assert_prose "\`devantler-tech/agent-skills\`** authors the bundled" \ "consumer does not name agent-skills as the owner of bundled skills" -assert_prose "${skill_path}\` carries" \ + +# The path, its provenance value and the non-authoring rule must appear in ONE paragraph. +# Asserting them independently against the whole contract is a scope hole: the specific rule +# could be deleted from this paragraph while a generic sentence elsewhere ("… not an +# authoring surface", about some other synced artifact) kept the guard green — reopening the +# misrouting regression this exists to block. +skill_para="$(awk -v p="${skill_path}" ' + index($0, p) { inpara = 1 } + inpara { if ($0 ~ /^[[:space:]]*$/) exit; print } +' "${constitution}" | tr '\n' ' ' | tr -s '[:space:]' ' ')" +[ -n "${skill_para}" ] || + fail "consumer does not name the bundled agent-improvement/SKILL.md as the synced copy" +assert_para() { + case "${skill_para}" in + *"$1"*) ;; + *) fail "$2" ;; + esac +} +assert_para "${skill_path}\` carries" \ "consumer does not name the bundled agent-improvement/SKILL.md as the synced copy" -assert_prose "It is a synced artifact, **not** an authoring surface" \ - "consumer does not mark the synced SKILL.md copy as a non-authoring surface" +assert_para 'github-repo: https://github.com/devantler-tech/agent-skills' \ + "the agent-improvement/SKILL.md paragraph does not name devantler-tech/agent-skills as its upstream" +assert_para 'It is a synced artifact, **not** an authoring surface' \ + "the agent-improvement/SKILL.md paragraph does not mark that copy a non-authoring surface" # Provenance is a per-FILE question: the same plugin directory holds synced skills and # locally-authored agents, so a per-directory rule is wrong in one direction or the other. @@ -81,11 +101,24 @@ assert_prose "It is a synced artifact, **not** an authoring surface" \ # ANCHOR THE VALUE'S END. An unanchored substring accepts any longer name with the same # prefix (`agent-skills-v2`), so it would miss exactly the ownership move it claims to # detect. Verified: the probe `…/agent-skills-v2` passes the unanchored form. -bundled_skill="${repo_root}/libraries/agent-plugins/plugins/agentic-engineering/skills/agent-improvement/SKILL.md" +# An UNINITIALISED submodule is a normal local state, not contract drift. Detect it first, or +# a fresh checkout reports "the skill is missing upstream" and hides the actionable fix. +plugin_root="${repo_root}/libraries/agent-plugins/plugins/agentic-engineering" +[ -d "${plugin_root}" ] || + fail "libraries/agent-plugins is not initialised, so the bundled skill cannot be checked. Initialise it with + .claude/scripts/submodule-init.sh libraries/agent-plugins" + +bundled_skill="${plugin_root}/skills/agent-improvement/SKILL.md" [ -f "${bundled_skill}" ] || fail "bundled agent-improvement/SKILL.md is missing at the pinned plugin revision — AGENTS.md routes generic skill edits through this path, so its absence invalidates the contract text" -grep -qE '^[[:space:]]*github-repo:[[:space:]]*https://github\.com/devantler-tech/agent-skills[[:space:]]*$' "${bundled_skill}" || - fail "bundled agent-improvement/SKILL.md no longer declares exactly devantler-tech/agent-skills as its upstream — re-check the owning repository before trusting the contract text" + +# Match inside the YAML FRONTMATTER only. Scanning the whole file would let the guard pass on +# a documentation example or code block in the body while the real metadata field was removed +# or repointed — the provenance would be gone and the guard would still claim to verify it. +skill_frontmatter="$(awk 'NR==1 && $0=="---" { infm=1; next } infm && $0=="---" { exit } infm { print }' "${bundled_skill}")" +printf '%s\n' "${skill_frontmatter}" | + grep -qE '^[[:space:]]*github-repo:[[:space:]]*https://github\.com/devantler-tech/agent-skills[[:space:]]*$' || + fail "bundled agent-improvement/SKILL.md frontmatter no longer declares exactly devantler-tech/agent-skills as its upstream — re-check the owning repository before trusting the contract text" # Every machine-readable entrypoint pointer must resolve to an agent the pinned plugin # actually BUNDLES. Derived from the submodule rather than hard-coded, so the next upstream From 920a7e72b0a7a5585b6e2e3b5bc78760d1402c79 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 26 Jul 2026 02:11:05 +0200 Subject: [PATCH 5/6] fix(agents): query provenance structurally and correct the upstream map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 4 of review found three more scope holes, each verified by the reviewer as passing before this change. Rather than patch the line-oriented checks a fourth time, this replaces them with a structural query — which closes the class and is simpler than what it removes. The provenance grep accepted the value under any mapping (`examples.github-repo`), accepted a body example after the real field was deleted, and treated frontmatter with no closing delimiter as valid. It now asks yq for the exact YAML path `metadata.github-repo`, which cannot express any of those confusions. The hand-rolled frontmatter extractor is gone. The owner assertion was still global, so the real owner could be changed while an unrelated copy of the phrase elsewhere kept CI green. Bullet extraction now starts at the owner line, binding owner, path, provenance value and the non-authoring rule to one block. Running the documented command to check it surfaced a real error in my own text: bundled skills come from several upstreams, not one. `find-skills` is from vercel-labs, `git-commit` and `refactor` from github/awesome-copilot, `test-driven-development` from obra/superpowers, `astro` from astrolicious. The contract said agent-skills owns the bundled skills, which would route a fix for any of those to a repository that does not own it — and each is a third party the ask-before-upstream rule covers. Corrected to name the value as the authority. One defect in this change's own first draft, caught and fixed: under `set -e` the yq call aborted the script silently on unparseable frontmatter, giving a failing test with no message. The failure is now captured and reported. RED-proven: value under another mapping, unterminated frontmatter, and the owner-swap-with-decoy each fail with their own message. Co-Authored-By: Claude Opus 5 (1M context) --- .../agent-role-delivery-contract.test.sh | 77 +++++++++---------- AGENTS.md | 34 +++++--- 2 files changed, 57 insertions(+), 54 deletions(-) diff --git a/.claude/scripts/agent-role-delivery-contract.test.sh b/.claude/scripts/agent-role-delivery-contract.test.sh index 291a7f37..8fadadd4 100755 --- a/.claude/scripts/agent-role-delivery-contract.test.sh +++ b/.claude/scripts/agent-role-delivery-contract.test.sh @@ -55,52 +55,35 @@ grep -Fq 'plugins/agentic-engineering/agents/agent-improver.agent.md' "${constit # is silently reverted. The consumer listed it as an authoring surface until 2026-07-25, # which would route a generic fix into a file that discards it. # -# Both assertions name the bundled skill PATH. Generic fragments ("authors the bundled", -# "not an authoring surface") could otherwise be satisfied by unrelated ownership prose -# elsewhere in the contract, which would let the specific routing rule be deleted while the -# guard stayed green. skill_path='plugins/agentic-engineering/skills/agent-improvement/SKILL.md' -assert_prose "\`devantler-tech/agent-skills\`** authors the bundled" \ - "consumer does not name agent-skills as the owner of bundled skills" -# The path, its provenance value and the non-authoring rule must appear in ONE paragraph. -# Asserting them independently against the whole contract is a scope hole: the specific rule -# could be deleted from this paragraph while a generic sentence elsewhere ("… not an -# authoring surface", about some other synced artifact) kept the guard green — reopening the -# misrouting regression this exists to block. -skill_para="$(awk -v p="${skill_path}" ' - index($0, p) { inpara = 1 } - inpara { if ($0 ~ /^[[:space:]]*$/) exit; print } +# Owner, path, provenance value and the non-authoring rule must all appear in ONE bullet. +# Asserting any of them against the whole contract is a scope hole — verified: changing the +# real owner to agent-plugins and appending an unrelated copy of the expected phrase +# elsewhere satisfied a global check. Extraction therefore starts at the OWNER line (the +# bullet's first line), not at the path line, so the owner declaration is bound to this skill. +skill_bullet="$(awk ' + /\*\*`devantler-tech\/agent-skills`\*\* authors `agent-improvement\/`/ { inb = 1 } + inb { if ($0 ~ /^[[:space:]]*$/) exit; print } ' "${constitution}" | tr '\n' ' ' | tr -s '[:space:]' ' ')" -[ -n "${skill_para}" ] || - fail "consumer does not name the bundled agent-improvement/SKILL.md as the synced copy" -assert_para() { - case "${skill_para}" in +[ -n "${skill_bullet}" ] || + fail "consumer does not name agent-skills as the owner of bundled skills" +assert_bullet() { + case "${skill_bullet}" in *"$1"*) ;; *) fail "$2" ;; esac } -assert_para "${skill_path}\` carries" \ - "consumer does not name the bundled agent-improvement/SKILL.md as the synced copy" -assert_para 'github-repo: https://github.com/devantler-tech/agent-skills' \ - "the agent-improvement/SKILL.md paragraph does not name devantler-tech/agent-skills as its upstream" -assert_para 'It is a synced artifact, **not** an authoring surface' \ - "the agent-improvement/SKILL.md paragraph does not mark that copy a non-authoring surface" +assert_bullet "${skill_path}\` carries" \ + "the agent-skills owner bullet does not name the bundled agent-improvement/SKILL.md" +assert_bullet 'github-repo: https://github.com/devantler-tech/agent-skills' \ + "the agent-skills owner bullet does not name devantler-tech/agent-skills as the upstream" +assert_bullet 'It is a synced artifact, **not** an authoring surface' \ + "the agent-skills owner bullet does not mark that copy a non-authoring surface" # Provenance is a per-FILE question: the same plugin directory holds synced skills and # locally-authored agents, so a per-directory rule is wrong in one direction or the other. # -# Match the github-repo KEY TOGETHER WITH ITS VALUE. A bare `grep -q github-repo` proves only -# that the file is synced from SOMEWHERE — it would stay green if the upstream moved to a -# different repository, which is exactly the case where the contract's routing text becomes -# wrong and this guard is the only thing that would notice. -# FAIL CLOSED on a missing file. A `[ -f ] &&` conditional would silently skip the whole -# assertion if the pinned plugin revision renamed or dropped the skill — leaving AGENTS.md -# routing edits through a path that no longer exists, with the contract test still green. -# -# ANCHOR THE VALUE'S END. An unanchored substring accepts any longer name with the same -# prefix (`agent-skills-v2`), so it would miss exactly the ownership move it claims to -# detect. Verified: the probe `…/agent-skills-v2` passes the unanchored form. # An UNINITIALISED submodule is a normal local state, not contract drift. Detect it first, or # a fresh checkout reports "the skill is missing upstream" and hides the actionable fix. plugin_root="${repo_root}/libraries/agent-plugins/plugins/agentic-engineering" @@ -112,13 +95,23 @@ bundled_skill="${plugin_root}/skills/agent-improvement/SKILL.md" [ -f "${bundled_skill}" ] || fail "bundled agent-improvement/SKILL.md is missing at the pinned plugin revision — AGENTS.md routes generic skill edits through this path, so its absence invalidates the contract text" -# Match inside the YAML FRONTMATTER only. Scanning the whole file would let the guard pass on -# a documentation example or code block in the body while the real metadata field was removed -# or repointed — the provenance would be gone and the guard would still claim to verify it. -skill_frontmatter="$(awk 'NR==1 && $0=="---" { infm=1; next } infm && $0=="---" { exit } infm { print }' "${bundled_skill}")" -printf '%s\n' "${skill_frontmatter}" | - grep -qE '^[[:space:]]*github-repo:[[:space:]]*https://github\.com/devantler-tech/agent-skills[[:space:]]*$' || - fail "bundled agent-improvement/SKILL.md frontmatter no longer declares exactly devantler-tech/agent-skills as its upstream — re-check the owning repository before trusting the contract text" +# Query the frontmatter STRUCTURALLY, at the exact YAML path `metadata.github-repo`. +# +# A line-oriented grep cannot express this and kept failing in new ways: it accepted the value +# under a different mapping (`examples.github-repo`), accepted a body example after the real +# field was deleted, and treated frontmatter with no closing delimiter as valid. yq resolves +# the real path or returns null, which is the property actually wanted — and it is SHORTER than +# the hand-rolled extraction it replaces, so this closes the hole while cutting complexity. +command -v yq >/dev/null || + fail "yq is required to verify the bundled skill's provenance structurally. Install it (brew install yq; it is preinstalled on GitHub ubuntu runners)" +# `|| skill_upstream=''` is load-bearing under `set -e`: yq exits non-zero on unparseable +# frontmatter (e.g. no closing delimiter), which would abort the script SILENTLY — a failing +# test with no message, indistinguishable from a crash. Capture the failure and let the +# assertion below report it with the actionable text. +skill_upstream="$(yq --front-matter=extract '.metadata.github-repo // ""' "${bundled_skill}" 2>/dev/null)" || + skill_upstream='' +[ "${skill_upstream}" = 'https://github.com/devantler-tech/agent-skills' ] || + fail "bundled agent-improvement/SKILL.md does not declare metadata.github-repo = https://github.com/devantler-tech/agent-skills (got: '${skill_upstream:-}') — re-check the owning repository before trusting the contract text" # Every machine-readable entrypoint pointer must resolve to an agent the pinned plugin # actually BUNDLES. Derived from the submodule rather than hard-coded, so the next upstream diff --git a/AGENTS.md b/AGENTS.md index b53e714c..52360c69 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -263,29 +263,39 @@ definition surface, and an installed/cache copy is never an authoring target. script, the provider-neutral desired state, plugin settings, and the Cursor loader source. The local Agent Improver agent/skill forks are retired, and so is the standalone FinOps agent fork — the reviewed plugin is the source for both roles. -- The generic upstream source, which is split across **two** repositories. **Check the file's own - provenance before editing it — the question is per-FILE, never per-directory**, because one plugin - directory mixes both kinds: +- The generic upstream source, which is **NOT one repository**. **Check the file's own provenance + before editing it — the question is per-FILE, never per-directory**, because one plugin directory + mixes locally-authored files with copies synced from *several different* upstreams: - **`devantler-tech/agent-plugins`** authors `plugins/agentic-engineering/agents/agent-improver.agent.md`, the plugin README/desired state, and their manifest/contract validation. These carry **no** `metadata.github-repo`. - - **`devantler-tech/agent-skills`** authors the bundled *skills*, including `agent-improvement/`. + - **`devantler-tech/agent-skills`** authors `agent-improvement/` and most of our own skills. 🔴 The copy at `plugins/agentic-engineering/skills/agent-improvement/SKILL.md` carries `metadata.github-repo: https://github.com/devantler-tech/agent-skills` and is re-pulled by the `update-agent-skills` workflow, so editing it there is **silently reverted** — no conflict, no CI failure, no signal. It is a synced artifact, **not** an authoring surface. - - Verify **from the monorepo root**, matching the key *and its value* — a bare `github-repo` test only - proves the file is synced from *somewhere*, not from the repo you are about to open a PR against: + - ⚠️ **Other bundled skills come from third-party upstreams entirely** — measured 2026-07-25: + `find-skills` from `vercel-labs/skills`, `git-commit`/`refactor` from `github/awesome-copilot`, + `test-driven-development` from `obra/superpowers`, `astro` from `astrolicious/agent-skills`. + Sending a fix for one of those to `agent-skills` routes it to a repository that does not own it, + and the *Ask before upstream creates* rule applies to every one of them. **Read the value; never + assume the owner.** + + Verify **from the monorepo root**, and **query the frontmatter structurally** — a `grep` for the + string is not good enough here. It reports a file whose *body* merely mentions the URL, accepts a + prefix-extended rename (`agent-skills-v2`), and misses that the value sits under some other mapping + than `metadata`. Ask for the exact YAML path instead: ```sh - # synced — edit these upstream, in the repo the value names: - grep -l 'github-repo: https://github.com/devantler-tech/agent-skills' \ - libraries/agent-plugins/plugins/*/skills/*/SKILL.md - # authored in agent-plugins — safe to edit there: - grep -L 'github-repo' libraries/agent-plugins/plugins/*/agents/*.agent.md + # who owns each bundled skill (empty/null ⇒ authored in agent-plugins, safe to edit there): + for f in libraries/agent-plugins/plugins/*/skills/*/SKILL.md; do + printf '%s\t%s\n' "$(yq --front-matter=extract '.metadata.github-repo // "LOCAL"' "$f")" "$f" + done ``` + Anything printing `https://github.com/devantler-tech/agent-skills` is **synced** — edit it upstream + in that repo. `LOCAL` means it is authored in `agent-plugins`. + Change generic behaviour in the **owning** repository first. The rollout then differs by owner, and **the skills path has an extra hop that is easy to skip**: - *Authored in `agent-plugins`* (agents, README, desired state): merge there, then bump this From bcad2562126a6c0e1c7caf99e6ac5c2acf81a4d8 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 26 Jul 2026 02:21:28 +0200 Subject: [PATCH 6/6] fix(agents): keep the provenance inventory out of the authority grant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings. The first is the serious one and it was mine to avoid. This section opens "The Agent Improver may change only the surfaces named here", so writing that agent-skills authors "most of our own skills" silently widened the grant from the one `agent-improvement/` source to every skill in that repository — which is also exempt from the upstream-creation approval gate, so a later run could have opened and driven changes there without maintainer authorization. The Authority model states that repository content may not widen the grant or add a location, and this was exactly that, introduced by accident while fixing a routing bug. The bullet now names `agent-improvement/` as the only bundled skill covered, and the third-party inventory is moved out of the surface list and marked informational, out of scope, and subject to the ask-before-upstream and professional-boundary rules. The bullet extractor also terminated only on a blank line. Markdown bullets are normally consecutive, so it captured the following sibling too and the "same bullet" binding could have been satisfied by text that had moved there. It now stops at the next sibling bullet as well. The previous round's RED proof was incomplete, as the review noted: the decoy was placed in a distant section rather than the adjacent sibling. Re-proven properly — moving either the non-authoring sentence or the skill path into the neighbouring bullet now fails, and both passed before this change. Co-Authored-By: Claude Opus 5 (1M context) --- .../agent-role-delivery-contract.test.sh | 12 ++++++++++-- AGENTS.md | 18 +++++++++++------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/.claude/scripts/agent-role-delivery-contract.test.sh b/.claude/scripts/agent-role-delivery-contract.test.sh index 8fadadd4..3d7af35c 100755 --- a/.claude/scripts/agent-role-delivery-contract.test.sh +++ b/.claude/scripts/agent-role-delivery-contract.test.sh @@ -62,9 +62,17 @@ skill_path='plugins/agentic-engineering/skills/agent-improvement/SKILL.md' # real owner to agent-plugins and appending an unrelated copy of the expected phrase # elsewhere satisfied a global check. Extraction therefore starts at the OWNER line (the # bullet's first line), not at the path line, so the owner declaration is bound to this skill. +# Stop at the next SIBLING BULLET as well as at a blank line. Markdown bullets are normally +# consecutive with no blank line between them, so a blank-line-only terminator swallowed the +# following bullet too — and the "same bullet" binding this guard claims could then be +# satisfied by text that had moved into that sibling. skill_bullet="$(awk ' - /\*\*`devantler-tech\/agent-skills`\*\* authors `agent-improvement\/`/ { inb = 1 } - inb { if ($0 ~ /^[[:space:]]*$/) exit; print } + !inb && /\*\*`devantler-tech\/agent-skills`\*\* authors `agent-improvement\/`/ { inb = 1; print; next } + inb { + if ($0 ~ /^[[:space:]]*$/) exit + if ($0 ~ /^[[:space:]]*[-*+] /) exit + print + } ' "${constitution}" | tr '\n' ' ' | tr -s '[:space:]' ' ')" [ -n "${skill_bullet}" ] || fail "consumer does not name agent-skills as the owner of bundled skills" diff --git a/AGENTS.md b/AGENTS.md index 52360c69..52cc9b45 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -269,17 +269,21 @@ definition surface, and an installed/cache copy is never an authoring target. - **`devantler-tech/agent-plugins`** authors `plugins/agentic-engineering/agents/agent-improver.agent.md`, the plugin README/desired state, and their manifest/contract validation. These carry **no** `metadata.github-repo`. - - **`devantler-tech/agent-skills`** authors `agent-improvement/` and most of our own skills. + - **`devantler-tech/agent-skills`** authors `agent-improvement/`, **and that one skill is the only + bundled skill this grant covers** — no other skill in that repository is a named surface. 🔴 The copy at `plugins/agentic-engineering/skills/agent-improvement/SKILL.md` carries `metadata.github-repo: https://github.com/devantler-tech/agent-skills` and is re-pulled by the `update-agent-skills` workflow, so editing it there is **silently reverted** — no conflict, no CI failure, no signal. It is a synced artifact, **not** an authoring surface. - - ⚠️ **Other bundled skills come from third-party upstreams entirely** — measured 2026-07-25: - `find-skills` from `vercel-labs/skills`, `git-commit`/`refactor` from `github/awesome-copilot`, - `test-driven-development` from `obra/superpowers`, `astro` from `astrolicious/agent-skills`. - Sending a fix for one of those to `agent-skills` routes it to a repository that does not own it, - and the *Ask before upstream creates* rule applies to every one of them. **Read the value; never - assume the owner.** + + ⚠️ **The following is INFORMATIONAL ROUTING GUIDANCE, not part of the grant.** It exists so a fix is + not sent to the wrong repository; it names **no** additional definition surface, and every skill in + it is **out of scope** for autonomous change. Other bundled skills come from third-party upstreams + entirely — measured 2026-07-25: `find-skills` from `vercel-labs/skills`, `git-commit`/`refactor` + from `github/awesome-copilot`, `test-driven-development` from `obra/superpowers`, `astro` from + `astrolicious/agent-skills`. Each is a third party, so the *Ask before upstream creates* rule and the + *Professional-work repository boundary* both apply before any interaction. **Read the value to learn + who owns a file; never read it as permission to change that file.** Verify **from the monorepo root**, and **query the frontmatter structurally** — a `grep` for the string is not good enough here. It reports a file whose *body* merely mentions the URL, accepts a