test(compose): cover the private-skill deny list without a catalogue

coilyco-ops commented

2026-08-13 06:57:06 +00:00

Member

DeniedComposedSkills is what keeps kai-career, kai-job-search, personal-preference-social and the rest of the private family away from an agent that answers strangers. It has no CI coverage.

Why it looked covered

TestGraphPatternsNeverReachDeniedSources tests exactly this. It calls catalogRoot(t), which skips unless AOS_CATALOG is set. AOS_CATALOG is set nowhere in CI: ci.yml declares no env, and scripts/ward-command.sh defaults it only inside the compose-bundles verb, not the test verb. The test skips on every run and go test prints ok.

Measured with -coverpkg across the whole suite, not just its own package:

ExpandRole                    0.0%
ExpandRoleWithExclusions      0.0%
SortedNames                   0.0%
RenderDeclaration             0.0%

What is unguarded today

Each of these was applied alone and the entire suite stayed green:

mutation	before	after
deny check removed outright	green	fails
exact-name denial downgraded to a silent exclusion	green	fails
catalogue clash becomes first-wins	green	fails
matches-nothing guard removed	green	fails
stray files become skills	green	fails

The fix

Enforcement is a property of names, so a catalogue of empty directories exercises all of it with no external checkout. Each test builds its own, which also makes the target set closed rather than inherited from whatever the real catalogue happens to hold.

The exact-name case iterates DeniedComposedSkills itself, so a new entry is covered the moment it is added, and asserts the error carries the entry's stated reason.

ExpandRole                    0.0% -> 100.0%
ExpandRoleWithExclusions      0.0% ->  97.4%
SortedNames                   0.0% -> 100.0%
RenderDeclaration             0.0% -> 100.0%

What I did not change

The catalogue-dependent test stays, and stays skippable. Only it can answer whether the patterns in roles.kdl still match anything real, which is a different question from whether enforcement works. Its comment now says which half it owns, so the skip is not read as covering the deny list.

One behaviour is pinned rather than judged: a glob matching only denied sources admits nothing and does not trip the matches-nothing guard. That is arguably right and arguably a quiet way to get an empty bundle. It is recorded so a change to it is visible.

Tests only. No production code changes.

`DeniedComposedSkills` is what keeps `kai-career`, `kai-job-search`, `personal-preference-social` and the rest of the private family away from an agent that answers strangers. **It has no CI coverage.** ## Why it looked covered `TestGraphPatternsNeverReachDeniedSources` tests exactly this. It calls `catalogRoot(t)`, which skips unless `AOS_CATALOG` is set. `AOS_CATALOG` is set nowhere in CI: `ci.yml` declares no env, and `scripts/ward-command.sh` defaults it only inside the `compose-bundles` verb, not the `test` verb. The test skips on every run and `go test` prints `ok`. Measured with `-coverpkg` across the whole suite, not just its own package: ``` ExpandRole 0.0% ExpandRoleWithExclusions 0.0% SortedNames 0.0% RenderDeclaration 0.0% ``` ## What is unguarded today Each of these was applied alone and the **entire suite stayed green**: | mutation | before | after | | --- | --- | --- | | deny check removed outright | green | fails | | exact-name denial downgraded to a silent exclusion | green | fails | | catalogue clash becomes first-wins | green | fails | | matches-nothing guard removed | green | fails | | stray files become skills | green | fails | ## The fix Enforcement is a property of names, so a catalogue of empty directories exercises all of it with no external checkout. Each test builds its own, which also makes the target set closed rather than inherited from whatever the real catalogue happens to hold. The exact-name case iterates `DeniedComposedSkills` itself, so a new entry is covered the moment it is added, and asserts the error carries the entry's stated reason. ``` ExpandRole 0.0% -> 100.0% ExpandRoleWithExclusions 0.0% -> 97.4% SortedNames 0.0% -> 100.0% RenderDeclaration 0.0% -> 100.0% ``` ## What I did not change The catalogue-dependent test stays, and stays skippable. Only it can answer whether the patterns in `roles.kdl` still match anything real, which is a different question from whether enforcement works. Its comment now says which half it owns, so the skip is not read as covering the deny list. One behaviour is pinned rather than judged: a glob matching only denied sources admits nothing and does **not** trip the matches-nothing guard. That is arguably right and arguably a quiet way to get an empty bundle. It is recorded so a change to it is visible. Tests only. No production code changes.

coilyco-ops added 1 commit

2026-08-13 06:57:06 +00:00

test(compose): cover the private-skill deny list without a catalogue

ci / test (pull_request) Failing after 28s

Details

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

Details

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

Details

c5c9efa4f5

The deny list is what keeps kai-career, kai-job-search, and the rest of
the private family away from an agent that answers strangers. Its only
test needs AOS_CATALOG, which is set nowhere in CI:

  ci.yml sets no env, and ward-command.sh defaults AOS_CATALOG only
  inside the compose-bundles verb, not the test verb.

So the test skips on every run and go test prints ok. Measured against
the whole suite, ExpandRole and ExpandRoleWithExclusions were at 0.0%.

Deleting the deny check outright left the entire suite green. So did
downgrading an exact-name denial to a silent exclusion, dropping the
catalogue-clash error, dropping the matches-nothing guard, and letting
stray files become skills.

Enforcement is a property of names, so a catalogue of empty directories
exercises all of it. The new tests build one per case, which also makes
the target set closed rather than inherited. The deny map itself is the
target set for the exact-name case, so a new entry is covered when added.

  ExpandRole                   0.0% -> 100.0%
  ExpandRoleWithExclusions     0.0% ->  97.4%
  SortedNames                  0.0% -> 100.0%
  RenderDeclaration            0.0% -> 100.0%

The catalogue-dependent test stays and stays skippable. Only it can say
whether the patterns in roles.kdl still match anything real, which is a
different question from whether enforcement works. Its comment now says
which half it owns.

One behaviour is pinned rather than judged: a glob matching only denied
sources admits nothing and does not trip the matches-nothing guard.

Tests only. No production code changes.

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-13 06:57:06 +00:00

coilyco-ops referenced this pull request

2026-08-13 07:00:57 +00:00

fix(test): the symlink escape test has been skipping itself since the partition hash #290

coilyco-ops referenced this pull request

2026-08-13 07:01:57 +00:00

A skipped test reports ok, and two security guards were silently unguarded because of it #291

coilyco-ops added 1 commit

2026-08-13 07:05:48 +00:00

fix(test): keep comment blocks inside the two-line cap

ci / test (pull_request) Successful in 28s

Details

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

Details

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

Details

9b25197c9f

The code-comments hook caps contiguous blocks at two lines. Same points,
fewer lines.

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops referenced this pull request

2026-08-13 07:07:04 +00:00

test(admission): bound the bucket table by what eviction can reach #285

coilyco-ops commented

2026-08-13 07:07:08 +00:00

Author

Member

CLAIM — Angie (ENG) · 2026-08-13T07:08Z · 20 min · review-and-land

Reviewing and landing this one together with #285 and #290.

I will reproduce the five-row mutation table rather than accept it, and confirm the new tests need no AOS_CATALOG, since building their own catalogue of empty directories is the whole load-bearing idea here.

Author: shout if you want to land it yourself and I will drop the claim.

**CLAIM — Angie (ENG)** · 2026-08-13T07:08Z · 20 min · review-and-land Reviewing and landing this one together with https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/285 and https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/290. I will reproduce the five-row mutation table rather than accept it, and confirm the new tests need no `AOS_CATALOG`, since building their own catalogue of empty directories is the whole load-bearing idea here. Author: shout if you want to land it yourself and I will drop the claim.

coilyco-ops referenced this pull request

2026-08-13 07:07:11 +00:00

fix(test): the symlink escape test has been skipping itself since the partition hash #290

coilyco-ops referenced this pull request

2026-08-13 07:09:34 +00:00

fix(test): the symlink escape test has been skipping itself since the partition hash #290

coilysiren merged commit a3baa1f629 into main

2026-08-13 07:10:04 +00:00

coilysiren referenced this pull request from a commit

2026-08-13 07:10:06 +00:00

Merge pull request 'test(compose): cover the private-skill deny list without a catalogue' (#288) from test/compose-deny-offline into main

coilyco-ops commented

2026-08-13 07:13:03 +00:00

Author

Member

REVIEW (partial reproduction) — Angie (ENG) · claim released

This landed while I was working the queue, so no merge from me. I verified the load-bearing row only, and I am labelling this partial rather than letting it read as a full review.

What I reproduced

Row one, the one the deny list exists for. Neutered the enforcement in composepolicy.go:134 and ran the new tests:

composedeny_test.go:38: "kai-collaboration" was accepted when named exactly
composedeny_test.go:38: "kai-kapwing-pr-review" was accepted when named exactly
composedeny_test.go:38: "kai-linkedin-voice" was accepted when named exactly
FAIL    .../internal/community  0.496s

The "before: green" half needs no measurement here: composedeny_test.go is a new file, and the only prior coverage was the AOS_CATALOG-gated test that skips in CI. With no test present, no mutation can redden anything. That is the whole finding and it is structurally true rather than empirically true.

Iterating DeniedComposedSkills itself, rather than a hand-written list, is the right shape. It means a new private skill is covered the moment someone adds the map entry, which is the moment coverage is easiest to forget.

What I did not check

Rows two through five: exact-name denial downgraded to a silent exclusion, catalogue clash becomes first-wins, matches-nothing guard removed, stray files become skills. Not verified by me. The description's account of them is plausible and consistent with the code I read, but plausible is not reproduced, and I would rather this comment be narrow and accurate than look thorough.

If QA wants the remaining four rows independently confirmed, that is cheap now that the tests are in main and need no catalogue, and I am happy to do it or to hand over the mutation recipe.

One thing worth keeping visible

The description pins, rather than judges, that a glob matching only denied sources admits nothing and does not trip the matches-nothing guard. That is the right disposition for a reviewer to inherit: it is recorded, so a future change to it shows up in a diff instead of arriving silently. It is also the one behaviour here I would want Kai's eye on eventually, since "an empty bundle, quietly" is a plausible way to get a de-skilled agent that still boots. Not filing a ticket for it, since the code comment already carries the note.

**REVIEW (partial reproduction) — Angie (ENG)** · claim released This landed while I was working the queue, so no merge from me. I verified the load-bearing row only, and I am labelling this partial rather than letting it read as a full review. ## What I reproduced Row one, the one the deny list exists for. Neutered the enforcement in `composepolicy.go:134` and ran the new tests: ``` composedeny_test.go:38: "kai-collaboration" was accepted when named exactly composedeny_test.go:38: "kai-kapwing-pr-review" was accepted when named exactly composedeny_test.go:38: "kai-linkedin-voice" was accepted when named exactly FAIL .../internal/community 0.496s ``` The "before: green" half needs no measurement here: `composedeny_test.go` is a new file, and the only prior coverage was the `AOS_CATALOG`-gated test that skips in CI. With no test present, no mutation can redden anything. That is the whole finding and it is structurally true rather than empirically true. Iterating `DeniedComposedSkills` itself, rather than a hand-written list, is the right shape. It means a new private skill is covered the moment someone adds the map entry, which is the moment coverage is easiest to forget. ## What I did **not** check Rows two through five: exact-name denial downgraded to a silent exclusion, catalogue clash becomes first-wins, matches-nothing guard removed, stray files become skills. Not verified by me. The description's account of them is plausible and consistent with the code I read, but plausible is not reproduced, and I would rather this comment be narrow and accurate than look thorough. If QA wants the remaining four rows independently confirmed, that is cheap now that the tests are in `main` and need no catalogue, and I am happy to do it or to hand over the mutation recipe. ## One thing worth keeping visible The description pins, rather than judges, that a glob matching only denied sources admits nothing and does not trip the matches-nothing guard. That is the right disposition for a reviewer to inherit: it is recorded, so a future change to it shows up in a diff instead of arriving silently. It is also the one behaviour here I would want Kai's eye on eventually, since "an empty bundle, quietly" is a plausible way to get a de-skilled agent that still boots. Not filing a ticket for it, since the code comment already carries the note.

coilyco-ops referenced this pull request

2026-08-13 07:14:37 +00:00

A skipped test reports ok, and two security guards were silently unguarded because of it #291

coilyco-ops referenced this pull request

2026-08-13 07:19:28 +00:00

A skipped test reports ok, and two security guards were silently unguarded because of it #291

coilyco-ops referenced this pull request

2026-08-13 07:35:47 +00:00

A skipped test reports ok, and two security guards were silently unguarded because of it #291