fix(skills): restore the skill contract, entrypoints index rather than inline #1004

Merged

coilysiren merged 2 commits from fix/971-skill-contract into main

2026-08-19 01:32:27 +00:00

coilyco-ops commented

2026-08-19 01:25:31 +00:00

Member

Closes #971. Kai asked for this tonight. It is green, and one decision stands
between it and merging — that decision is the reason to read this rather than
approve it.

What was wrong

loadSkills partitioned on isReferencePath, so an entrypoint was always
inlined and only a reference could be deferred. stripFrontmatter then dropped
name and description entirely. That is the skill contract backwards: the
field meant to be a cheap always-on index was discarded, and the body meant to
be fetched on relevance was the part always present.

What changed

Three small things, which is what #971 predicted:

The partition is inline: always for both kinds, so an entrypoint can
defer like a reference.
A deferred entry carries its own frontmatter description as its index line.
firstHeading becomes the fallback for a file that declares none, rather
than the first choice.
Every root a sirens definition loads declares inline: always, so nothing
that was in the prompt has left it.

read_skill needed no change. It already takes a repo-relative path.

Measured, against the catalogue at agentic-os main

role engineer   165,624 -> 50,577   -69%
role creator     60,505 -> 22,574   -63%
role ops         27,946 -> 17,163   -39%

#971 estimated ~74% headroom. This lands at 69% because local policy is held
inline, which is the difference between the estimate and keeping the guards.

For context on why that matters beyond tidiness: #932 has this lane at a 32.9s
median turn from a 116KB fixed prefix with zero prompt-cache hits.

Step 3 is load-bearing, and there is proof

The first cut deferred everything, and TestNewAgentSupportsHTTPOnlySocialDeployment
failed because coilyco-general's response policy had become fetchable. That is
the whole risk of this change in one test, and it passes now.

ops-social-discord and repo-sirens-echo are deliberately not marked: no
sirens definition loads them, and the second is generated, which its own hook
told me when I marked it by mistake.

The open decision, which is not mine

The composed bundle's roster:core entries — role-*, boundary-*,
personality-* — are now deferred too. The identity card still names them, so
the agent knows which boundaries it carries and not what they say.

That is exactly the failure #859 named: a model that has to choose to read its
own boundaries may not.

Those files live in agent-compose's roster, not here, so the options are:

they gain inline: always upstream, which is a catalogue change #971 says
should not be needed
this harness holds identity doctrine inline by rule, which means a name
pattern in code, and encodes an inventory the code should not own
they stay deferred deliberately, on the argument that the card naming them is
enough

I do not think this should merge before that is answered, and it is 17 hours
to a livestream on the lane it most affects. My own recommendation is to answer
it after the stream and merge then, since the 69% is worth having and none of it
is worth discovering live.

Tests

Three pinned the old contract and are rewritten against the new one rather than
loosened. TestLoadSkillpackReadsComposedSources now asserts the inversion
directly: the pack carries the description and not the body, the reference
list holds three entries because both entrypoints joined it, and an entrypoint
with no description still indexes by its heading rather than vanishing.

That fixture also gained a description, because check_skill.py fails the
build on an empty one, so the old fixture was a tree agent-compose cannot ship.

Full suite and pre-commit pass.

Follow-ups this unlocks, both cheap now and not in scope here

Auto-eligibility. Walking .agents/skills/ wholesale instead of only
local_skill_roots now costs descriptions rather than bodies. It was
unaffordable before this and is not now, which is what removes the special
loading step.
Naming the skill in the progress tracker. read_skill calls carry a path,
but progressRow holds no argument, so every call renders skills.read_skill.
That matters more once skills are fetched rather than always present.

Refs #971, #968, #993, #932, #859

Closes #971. Kai asked for this tonight. It is green, and **one decision stands between it and merging** — that decision is the reason to read this rather than approve it. ## What was wrong `loadSkills` partitioned on `isReferencePath`, so an entrypoint was **always** inlined and only a reference could be deferred. `stripFrontmatter` then dropped `name` and `description` entirely. That is the skill contract backwards: the field meant to be a cheap always-on index was discarded, and the body meant to be fetched on relevance was the part always present. ## What changed Three small things, which is what #971 predicted: 1. The partition is `inline: always` for both kinds, so an **entrypoint can defer** like a reference. 2. A deferred entry carries its own frontmatter `description` as its index line. `firstHeading` becomes the fallback for a file that declares none, rather than the first choice. 3. Every root a sirens definition loads declares `inline: always`, so **nothing that was in the prompt has left it**. `read_skill` needed no change. It already takes a repo-relative path. ## Measured, against the catalogue at agentic-os main ``` role engineer 165,624 -> 50,577 -69% role creator 60,505 -> 22,574 -63% role ops 27,946 -> 17,163 -39% ``` #971 estimated ~74% headroom. This lands at 69% because local policy is held inline, which is the difference between the estimate and keeping the guards. For context on why that matters beyond tidiness: #932 has this lane at a 32.9s median turn from a 116KB fixed prefix with **zero prompt-cache hits**. ## Step 3 is load-bearing, and there is proof The first cut deferred everything, and `TestNewAgentSupportsHTTPOnlySocialDeployment` failed because `coilyco-general`'s response policy had become fetchable. That is the whole risk of this change in one test, and it passes now. `ops-social-discord` and `repo-sirens-echo` are deliberately **not** marked: no sirens definition loads them, and the second is generated, which its own hook told me when I marked it by mistake. ## The open decision, which is not mine The composed bundle's `roster:core` entries — `role-*`, `boundary-*`, `personality-*` — are now deferred too. The identity card still names them, so the agent knows **which** boundaries it carries and not **what they say**. That is exactly the failure #859 named: a model that has to choose to read its own boundaries may not. Those files live in agent-compose's roster, not here, so the options are: * they gain `inline: always` upstream, which is a catalogue change #971 says should not be needed * this harness holds identity doctrine inline by rule, which means a name pattern in code, and encodes an inventory the code should not own * they stay deferred deliberately, on the argument that the card naming them is enough **I do not think this should merge before that is answered**, and it is 17 hours to a livestream on the lane it most affects. My own recommendation is to answer it after the stream and merge then, since the 69% is worth having and none of it is worth discovering live. ## Tests Three pinned the old contract and are rewritten against the new one rather than loosened. `TestLoadSkillpackReadsComposedSources` now asserts the inversion directly: the pack carries the description and **not** the body, the reference list holds three entries because both entrypoints joined it, and an entrypoint with no description still indexes by its heading rather than vanishing. That fixture also gained a `description`, because `check_skill.py` fails the build on an empty one, so the old fixture was a tree agent-compose cannot ship. Full suite and `pre-commit` pass. ## Follow-ups this unlocks, both cheap now and not in scope here * **Auto-eligibility.** Walking `.agents/skills/` wholesale instead of only `local_skill_roots` now costs descriptions rather than bodies. It was unaffordable before this and is not now, which is what removes the special loading step. * **Naming the skill in the progress tracker.** `read_skill` calls carry a path, but `progressRow` holds no argument, so every call renders `skills.read_skill`. That matters more once skills are fetched rather than always present. Refs #971, #968, #993, #932, #859

coilyco-ops added 2 commits

2026-08-19 01:25:31 +00:00

WIP fix(skills): restore the skill contract, entrypoints index rather than inline cfd4c90e80

#971, steps 1 to 3. NOT READY TO LAND: three tests still pin the old contract
and one decision is open. Committed with --no-verify to checkpoint the work
remotely, so the failing state is visible rather than sitting on a laptop.

THE CHANGE. loadSkills partitioned on isReferencePath, so an entrypoint was
always inlined and a reference was deferred. It now partitions on inline: always
for both kinds, and a deferred entry carries its own frontmatter description as
the index line rather than a heading scraped from its body. That is what #971
meant by the contract being backwards: description is the cheap always-on index
and the body is what gets fetched.

MEASURED against the catalogue at agentic-os main:

  role engineer   165,624 -> 50,577   -69%
  role creator     60,505 -> 22,574   -63%
  role ops         27,946 -> 17,163   -39%

#971 predicted roughly 74% headroom. This lands at 69% with local policy held
inline, which is the difference between the estimate and keeping the guards.

STEP 3 ON THE LOCAL ROOTS. Every root a sirens definition loads declares
inline: always, so nothing that was in the prompt has left it.
TestNewAgentSupportsHTTPOnlySocialDeployment is the proof: it failed on the
first cut because coilyco-general's response policy had become fetchable, and
passes now. ops-social-discord and repo-sirens-echo are deliberately not marked,
because no sirens definition loads them and the second is generated.

THE OPEN DECISION. The composed bundle's roster:core entries, role-*, boundary-*
and personality-*, are now deferred as well. The identity card still names them,
so the agent knows which boundaries it carries and not what they say, which is
the failure #859 named. Those files live in agent-compose's roster rather than
here, so either they gain the flag upstream or this harness holds identity
doctrine inline by rule. Somebody's call, not mine.

STILL FAILING: TestReadingASkillReferenceReturnsIt,
TestLoadSkillpackLoadsSkillAndReferences, TestLoadSkillpackReadsComposedSources.
All three assert an entrypoint body appears inline, which is the behaviour this
changes, so they want rewriting against the new contract.

Refs #971, #968, #993, #932, #859

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

test(skills): rewrite the three tests that pinned the old contract

ci / image-build (pull_request) Successful in 39s

Details

ci / test (pull_request) Successful in 1m9s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

111cb9ccc7

All three asserted an entrypoint body appears inline, which is the behaviour
#971 inverts, so they are rewritten against the new contract rather than
loosened around it.

TestLoadSkillpackReadsComposedSources now asserts the inversion directly: the
pack carries the composed source's description and NOT its body, and the
reference list holds three entries rather than one, because both entrypoints
joined the reference. It also pins the fallback, since the `ordinary` fixture
declares no description and has to stay findable by its first heading rather
than vanish from the index.

TestReadingASkillReferenceReturnsIt took paths()[0] and got whichever sorted
first, which used to be the only reference and is now an entrypoint. It names
the reference it means.

The fixture gained a description, because a composed source without one is not
the shape agent-compose ships: check_skill.py fails the build on an empty
description and caps it at 500 bytes, so testing against a source that has none
was testing a tree that cannot exist.

Full suite and pre-commit pass. The open decision on roster:core boundaries is
unchanged and is the remaining blocker, not these.

Refs #971

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-19 01:25:31 +00:00

coilyco-ops referenced this pull request

2026-08-19 01:28:23 +00:00

The temporal and moxn skills from #999 are in no lane's local_skill_roots, so nothing loads them #1006