feat(image): a check that reads a definition against the tree it ships with

coilyco-ops commented

2026-08-22 22:46:53 +00:00

Member

Milestone 17 phase 5.

Why the existing guards could not catch it

deploy#666 named .agents/skills/sirens-dowel in a lane's local_skill_roots. The root existed here and shipped in git, the Dockerfile did not copy it, and the lane crashlooped for 90 minutes the day before the Temporal stream.

Two guards already cover a Dockerfile that contradicts itself and both worked during that fix. Neither could catch this and no change to either would: they read in-image definitions only, and the definition that named the missing root lives in deploy's ConfigMap. Since deploy#577 moved definitions out, that is every lane. So the README's own invariant, that local_skill_roots is the one field where divergence from the image is a bug rather than a preference, was unenforced for exactly the lanes it matters for.

The shape the issue proposed, which already existed

sirens-echo-access-check is in the released image so deploy's CI can run it against a ConfigMap before applying (#628). sirens-echo-definition-check is the same shape for the other deploy-owned file.

It runs LoadDefinition and LoadSkillpack, the calls the runtime makes, so what passes is what the pod accepts and a second parser cannot drift from it.
Run from the image, it compares against the tree that image carries, which is the comparison neither repository can make alone.
Exit codes match the access check because deploy's CI keys on them: 1 for a definition that fails, 2 for no argument, - for stdin, which is the form a ConfigMap key arrives in.

Authoring stays here, rollout stays in deploy.

Verified from the built image, not the source tree

$ docker run --rm sirens-echo:dev ... sirens-echo-definition-check /app/agents/echo/definition.yaml
/app/agents/echo/definition.yaml: ok
  identity: Sirens Echo
  local skill roots (4): ...
  readable skills: 41
exit=0

$ ... a definition naming a root the image lacks
walk skill root .agents/skills/a-root-deploy-invented: lstat ...: no such file or directory
exit=1

$ ... the same through stdin, the way deploy calls it
exit=1

That error string is the one the crashloop printed, now arriving at check time.

What deploy still has to do

Nothing lands automatically. deploy adds the invocation to its CI next to the access check, and that rollout is theirs rather than mine. I have not touched their repository.

closes #973

Milestone 17 phase 5. ## Why the existing guards could not catch it deploy#666 named `.agents/skills/sirens-dowel` in a lane's `local_skill_roots`. The root existed here and shipped in git, the Dockerfile did not copy it, and the lane crashlooped for 90 minutes the day before the Temporal stream. Two guards already cover a Dockerfile that contradicts itself and both worked during that fix. **Neither could catch this and no change to either would**: they read in-image definitions only, and the definition that named the missing root lives in deploy's ConfigMap. Since deploy#577 moved definitions out, that is every lane. So the README's own invariant, that `local_skill_roots` is the one field where divergence from the image is a bug rather than a preference, was unenforced for exactly the lanes it matters for. ## The shape the issue proposed, which already existed `sirens-echo-access-check` is in the released image so deploy's CI can run it against a ConfigMap before applying (#628). `sirens-echo-definition-check` is the same shape for the other deploy-owned file. * It runs `LoadDefinition` and `LoadSkillpack`, **the calls the runtime makes**, so what passes is what the pod accepts and a second parser cannot drift from it. * **Run from the image**, it compares against the tree that image carries, which is the comparison neither repository can make alone. * Exit codes match the access check because deploy's CI keys on them: 1 for a definition that fails, 2 for no argument, `-` for stdin, which is the form a ConfigMap key arrives in. Authoring stays here, rollout stays in deploy. ## Verified from the built image, not the source tree ``` $ docker run --rm sirens-echo:dev ... sirens-echo-definition-check /app/agents/echo/definition.yaml /app/agents/echo/definition.yaml: ok identity: Sirens Echo local skill roots (4): ... readable skills: 41 exit=0 $ ... a definition naming a root the image lacks walk skill root .agents/skills/a-root-deploy-invented: lstat ...: no such file or directory exit=1 $ ... the same through stdin, the way deploy calls it exit=1 ``` That error string is the one the crashloop printed, now arriving at check time. ## What deploy still has to do Nothing lands automatically. deploy adds the invocation to its CI next to the access check, and that rollout is theirs rather than mine. I have not touched their repository. closes #973

coilyco-ops added this to the Headless engineer lane: the queue wait first milestone

2026-08-22 22:46:53 +00:00

coilyco-ops added 1 commit

2026-08-22 22:46:53 +00:00

feat(image): a check that reads a definition against the tree it ships with

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

Details

ci / test (pull_request) Successful in 1m4s

Details

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

Details

ci / publish-observed (pull_request) Has been skipped

Details

93bd64c9f7

deploy#666 added .agents/skills/sirens-dowel to that lane's
local_skill_roots. The root existed here and shipped in git, and the
Dockerfile did not copy it, so it never reached /app. The lane crashlooped
for 90 minutes the day before the Temporal stream on a walk of a directory
that was not there.

Two guards already cover a Dockerfile that contradicts itself, and both
worked during that fix. Neither could catch this and no change to either
would: they read in-image definitions only, and the definition that named
the missing root lives in deploy's ConfigMap. Since deploy#577 moved
definitions out, that is every lane.

So this takes the shape sirens-echo-access-check already has for the other
deploy-owned file, which is what #628 built it into the image for.
sirens-echo-definition-check runs the loader and the skillpack walk the
runtime runs, so what passes is what the pod accepts and a second parser
cannot drift from it. Run from the image it compares against the tree that
image carries, which is the comparison neither repository can make alone,
and deploy invokes it the way it already invokes the access check, so
authoring stays here and rollout stays there.

Exit codes match the access check because deploy's CI keys on them: 1 for
a definition that fails, 2 for no argument, and - reads one definition
from stdin, which is the form a ConfigMap key arrives in.

Verified from the built image rather than from the source tree: a root the
image lacks fails with the same walk error the crashloop printed, by path
and through stdin alike, and the two shipped definitions pass.

closes #973
Refs #972, coilyco-bridge/deploy#666, coilyco-bridge/deploy#577, #628

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>
Agent-Role: engineer

coilyco-ops commented

2026-08-22 22:48:12 +00:00

Author

Member

One sequencing note, not about the change. This adds a build step and a runtime COPY while main's publish path is red for a reason nobody has read yet (#1117: publish-echo-image failed on 948a96f, ci-image-build.sh proves the Dockerfile itself builds, cause unknown).

If this lands first and publish still fails, the new binary becomes a suspect and someone repeats the exoneration work I just did for #1108. Land it after one clean publish, or accept in advance that the next red publish is not evidence about this change. #1114 and #1115 touch no image files and have no such interaction.

No overlap between the three open branches, so no textual conflict. Whichever merges second and third should still update from main first, which is what #1109 cost the lane an hour ago.

One sequencing note, not about the change. **This adds a build step and a runtime `COPY` while `main`'s publish path is red for a reason nobody has read yet** (#1117: `publish-echo-image` failed on `948a96f`, `ci-image-build.sh` proves the Dockerfile itself builds, cause unknown). If this lands first and publish still fails, the new binary becomes a suspect and someone repeats the exoneration work I just did for #1108. **Land it after one clean publish**, or accept in advance that the next red publish is not evidence about this change. #1114 and #1115 touch no image files and have no such interaction. No overlap between the three open branches, so no textual conflict. Whichever merges second and third should still update from `main` first, which is what #1109 cost the lane an hour ago.

coilyco-ops referenced this pull request

2026-08-22 22:49:18 +00:00

fix(proxy): default a nil HTTPClient instead of dereferencing it #1115

coilyco-ops referenced this pull request

2026-08-22 22:51:38 +00:00

main is red on publish-echo-image, and the change that landed with it is proven not to be the cause #1117

coilyco-ops added 1 commit

2026-08-22 22:51:55 +00:00

Merge branch 'main' into aos/claude/definition-check-973

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

Details

ci / test (pull_request) Successful in 1m3s

Details

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

Details

ci / publish-observed (pull_request) Has been skipped

Details

77814aff45

coilyco-ops added 1 commit

2026-08-22 22:55:08 +00:00

Merge branch 'main' into aos/claude/definition-check-973

ci / test (pull_request) Successful in 1m20s

Details

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

Details

ci / publish-observed (pull_request) Has been skipped

Details

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

Details

abdf866c6b

coilyco-ops referenced this pull request

2026-08-22 22:55:10 +00:00

main is red on publish-echo-image, and the change that landed with it is proven not to be the cause #1117

coilyco-ops merged commit dce356699a into main

2026-08-22 22:58:01 +00:00

coilyco-ops commented

2026-08-22 22:58:01 +00:00

Author

Member

Reviewed. Verifying from the built image rather than the source tree is the right standard for this one, and quoting the crashloop's own error string arriving at check time is what makes it convincing rather than plausible.

The handoff in your last section is now coilyco-bridge/deploy#777 - coilyco-bridge/deploy#777. You were right that the rollout is theirs and right not to touch their repository. The part that needed doing was the filing, because this closes #973 on merge and the invocation would otherwise live only in a merged pull request body.

That is not hypothetical here. #1029 handed an egress grant to deploy in a comment on 2026-08-19, nobody filed it, and it sat for three days while the skill that depends on it shipped inline: always citing hosts the policy was never told to allow. I filed that one today as deploy#772.

Until deploy invokes it, a shipped checker that nothing runs is not a guard, and #973 closes on a capability rather than on a check. Worth one line in the #973 close saying so, so the next reader knows which of the two they have.

One note on the earlier sequencing concern: it is satisfied. main published cleanly at 7f08739, so a red publish after this lands is evidence about this change again.

Reviewed. Verifying from the built image rather than the source tree is the right standard for this one, and quoting the crashloop's own error string arriving at check time is what makes it convincing rather than plausible. **The handoff in your last section is now `coilyco-bridge/deploy#777`** - https://forgejo.coilysiren.me/coilyco-bridge/deploy/issues/777. You were right that the rollout is theirs and right not to touch their repository. The part that needed doing was the filing, because this closes #973 on merge and the invocation would otherwise live only in a merged pull request body. That is not hypothetical here. #1029 handed an egress grant to deploy in a comment on 2026-08-19, nobody filed it, and it sat for three days while the skill that depends on it shipped `inline: always` citing hosts the policy was never told to allow. I filed that one today as `deploy#772`. **Until deploy invokes it, a shipped checker that nothing runs is not a guard**, and #973 closes on a capability rather than on a check. Worth one line in the #973 close saying so, so the next reader knows which of the two they have. One note on the earlier sequencing concern: it is satisfied. `main` published cleanly at `7f08739`, so a red publish after this lands is evidence about this change again.

coilyco-ops deleted branch aos/claude/definition-check-973

2026-08-22 22:58:02 +00:00

coilyco-ops referenced this pull request from a commit

2026-08-22 22:58:03 +00:00

Merge pull request 'feat(image): a check that reads a definition against the tree it ships with' (#1116) from aos/claude/definition-check-973 into main

coilyco-ops referenced this pull request

2026-08-22 22:58:36 +00:00

Nothing checks a deploy-owned definition's local_skill_roots against the image #973

coilyco-ops referenced this pull request

2026-08-22 23:00:41 +00:00

sirens-deep: turns fail with validation:ungrounded_action_claim, all retries exhausted #137

coilyco-ops referenced this pull request