Skip to content

test(BA-7837): write the image adapter scenarios and their tests - #14520

Draft
jopemachine wants to merge 12 commits into
mainfrom
docs/BA-7837-image
Draft

test(BA-7837): write the image adapter scenarios and their tests#14520
jopemachine wants to merge 12 commits into
mainfrom
docs/BA-7837-image

Conversation

@jopemachine

@jopemachine jopemachine commented Sep 11, 2026

Copy link
Copy Markdown
Member

Split out of #14503, which held both entities of this concern. The container registry adapter is #14519.

Summary

  • Replace the TODO stub in api/adapters/image/KNOWLEDGE.md with what the adapter guarantees, in the shape adapters/domain/KNOWLEDGE.md set in test(BA-7779): PoC of scenario-table tests through the adapter layer #14448, and transcribe it into tests/scenario/bai_scenario/manager/image/ — 43 rows over six modules for the eleven calls it offers.
  • Extend SeedImage with the status and the customized-owner pair, and add SeedAlias, so a scenario can lay an image its caller owns.
  • The role that reaches an image sits on its registry, because that is the scope an image is created under. The agent registry and the image cache stay unwired: none of the eleven calls read them.

Coverage

Call Rows
admin_search, admin_search_images_gql, admin_search_image_aliases 9
batch_load_by_ids, batch_load_aliases_by_ids 5
admin_forget, admin_restore 11
admin_purge 6
admin_alias, admin_dealias 7
admin_update 5

What a reviewer should look at

The document is the claim; the tests are its transcription, and CI checks the transcription against a real database. Reading KNOWLEDGE.md (140 lines) plus the coverage table above is the review.

This adapter splits its calls across three gates, and the third is the one worth checking:

  • The superadmin role guards searching, aliasing and editing.
  • The entity permission guards forgetting, restoring and purging.
  • An ownership check runs after that gate, and the rbac enforcement switch does not reach it. An image is owned only when it is customized and the caller made it, so a shipped image refuses every caller but a superadmin — even one holding permission on it, and even with enforcement off.

Two rows say what the run does rather than what the code looked like it would do, and both look like bugs worth their own issue rather than something to fix here:

  • Restoring cannot reach a forgotten image. It is looked up where only a live image is visible, so admin_restore answers that the image is not there. Only an image that was never forgotten can be "restored", which changes nothing.
  • Forgetting cannot reach an image a purge is working through, for the same reason, so the check that refuses a status write mid-purge is unreachable through this adapter.

Test plan

  • The scenario suite for this entity is green in CI
  • report.md generated from the run and committed

Resolves BA-7837

🤖 Generated with Claude Code

https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB

@jopemachine @claude
test(BA-7837): write the image adapter scenarios and their tests
27f9401 
Replace the TODO stub in `api/adapters/image/KNOWLEDGE.md` with the scenarios the
adapter guarantees, following the shape `adapters/domain/KNOWLEDGE.md` set, and
transcribe every row into `tests/scenario`: 43 rows over six modules for the
eleven calls it offers.

- Extend `SeedImage` with the status and the customized-owner pair, and add
  `SeedAlias`, so a scenario can lay an image its caller owns.
- The role that reaches an image sits on its registry, because that is the scope
  an image is created under.
- Leave the agent registry and the image cache unwired: none of the eleven calls
  read them, and a scenario that turns out to need one fails saying so.

The rows that matter most are the two where a granted caller is still refused an
image nobody owns, and where turning enforcement off opens the gate but not the
ownership check behind it. Two more state what the run does rather than what the
code looked like it would do: restoring cannot reach a forgotten image, and
forgetting cannot reach one a purge is working through, because both are looked
up where only a live image is visible.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB
@github-actions github-actions Bot added the size:XL 500~ LoC label Sep 11, 2026
@jopemachine jopemachine mentioned this pull request Sep 11, 2026
Draft
2 tasks
@jopemachine jopemachine mentioned this pull request Sep 11, 2026
Closed
2 tasks
jopemachine and others added 10 commits September 11, 2026 13:15
@jopemachine @claude
test(BA-7837): record what the run covers, and name the call it does not
5d09941 
The report is generated from the run rather than written, so it cannot drift:
43 rows, every one passing, each with the rows it laid and what it saw.

It also counts the calls no scenario reached, and named one the document had
claimed was covered — `batch_load_fields`, which the adapter base offers rather
than this adapter. 아직 적지 않은 것 now says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB
@jopemachine @claude
test(BA-7837): name a scenario by what it does, not by how it ends
2cb25fe 
Several scenarios carried only their outcome — `TheSuperadminForgets`,
`TheAliasGoesWithIt`, `AnAliasIsAttached` — which is what the `then` of the row
already says, and reads as a name for one. Each `summary` next to them was
already the whole claim, so the class names were the degraded half.

The rule the rest of them follow: a scenario is named by the operation or the
situation it drives, and an answer keeps its own name. Thirteen names move to
match; `summary` is untouched, so the report and its rows are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB
@jopemachine @claude
test(BA-7837): make two rows check what they say they check
f390cda 
The row about a condition given from outside narrowed a search to the only
registry there was, so nothing was excluded and it would have passed with the
condition removed. It now lays images in two registries and counts only the one
it narrowed to.

Alias search had a row for the empty answer and one for the refusal, and none
for finding an alias that is there — so it would have passed with the search
returning nothing at all. Adds that row.

The row about purging taking the aliases with it cannot see that happen: the
answer carries the image and nothing else. Its sentence now claims only what it
looks at, and 아직 적지 않은 것 says why the rest is not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB
@jopemachine @claude
docs(BA-7837): bring the report level with the run
e2053fc 
Regenerated from the run: 44 rows, every one passing. It picks up the row added
for finding an alias that is there, the narrowing row now laying two registries,
and the sentences that stopped claiming what they cannot see.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB
@jopemachine @claude
test(BA-7837): look at every field of the image node
99b059c 
`tests/scenario/AGENTS.md` says a check never looks at part of the answer.
Four checkers looked at 5-9 of the 22 fields `ImageNode` carries, so the rest
could change without a scenario noticing.

- `TheImageNode` now answers every one of the 22, descending into `identity`,
  `metadata` and `requirements`. The default resource limits come from
  `INTRINSIC_SLOTS_MIN` rather than a literal, and both limit lists are sorted
  by slot so the report reads the same on every run.
- `TheTagIsTheOnlyChange`, `NoAcceleratorsAreLeft` and `TheRetiredImageComesBack`
  were the same check with one field moved; they are gone and their rows take
  `TheImageNode(tag=...)`.
- A row that expects an answer took `Refused(type(answered.raised), ...)` when
  the call refused, which passes for any exception. It now holds the answer to
  `Filled`, so an unexpected refusal fails.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB
@jopemachine @claude
test(BA-7837): let the accelerator rows observe a change
f613fe0 
`KNOWLEDGE.md` says the accelerator list takes three asks — omit, clear, name a
value — and that the clearing row runs against an image that holds one. Neither
held: `SeedImage` could not plant an accelerator, so the clearing row cleared a
slot that was already empty and proved nothing, and no row named a value at all.

- `SeedImage` takes `accelerators`, and `Accelerators` (`NoAccelerator` /
  `OneAccelerator`) carries the value through the given, the ask and the answer
  in place of the `clearing_accelerators` flag.
- The clearing row now plants an accelerator and watches it go.
- A row for the third ask, naming a value onto an image that had none, joins the
  table in `KNOWLEDGE.md` and in the scenarios.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB
@jopemachine @claude
test(BA-7837): say what the rows actually plant
933dad9 
Three rows described a situation they did not set up, and one verdict was named
after a field it did not look at.

- The two restore rows and the ungranted-restore row planted a live image and a
  superadmin; `KNOWLEDGE.md` claimed a forgotten image and a per-image grant.
  A forgotten image would answer `ImageNotFound` instead, so the tables move to
  what is there.
- `OnePageComesBack` called its verdict `items` while holding the page's length.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB
@jopemachine @claude
test(BA-7837): record the two values the node does not hand back as s…
c940b83 
…tored

Looking at every field turned up two of them.

- `config_digest` and `metadata.digest` come back space-padded: the column is a
  fixed 72 wide and a real digest is 71, so every answer carries one trailing
  space. The check holds the digest to the planted value plus padding.
- `registry_id` arrives wrapped in `ArtifactRegistryID` while the image belongs
  to a container registry. `EntityIdentifier` equality includes the kind, so it
  never equals the `ContainerRegistryID` the row was written with; the check
  compares the uuid.

Both are written down in `KNOWLEDGE.md` as what happens today.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB
@jopemachine @claude
docs(BA-7837): write the Korean as plain technical prose
acb4c6e 
The scenario text followed the metaphor-heavy style of the framework docs —
문/줄/자리/심다/닿다/실리다/답으로 온다 — which reads as a puzzle rather than a
description. Every Korean sentence in `KNOWLEDGE.md`, the `describe`/`says`/
`detail` strings and the docstrings is rewritten against the agreed vocabulary:
생성/조회/검색/수정/소프트 삭제/복원/완전 삭제, 권한 검사, 역할 부족으로 거부,
대상을 찾을 수 없어 거부, 미리 만들어 둔, 반환된다, 담긴, 집계된다, 하위 계층.
Counts are Arabic numerals and no particle follows an English identifier.

Two tables in `KNOWLEDGE.md` also drifted from the scenarios they describe: the
alias search was missing the row for an empty result, and four 상황 cells named a
setup the scenario does not build. The document now lists 45 rows against the 45
scenarios that run.

Phrases the framework itself prints are left alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB
@jopemachine @claude
docs(BA-7837): bring the report level with the run
6b5283d 
45 scenarios, all passing. Each full-node check now prints all 31 places it
looks at, so the report shows what the answer actually carries — the accelerator
leaving the image, the padded digest, the derived resource limits.

One condition named a field with a particle attached straight to the identifier;
it now ends on a Korean word.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TYyokzEfpr9AF2NnHtyXB
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot will automatically review once the pull request is marked ready for review

At least 1 approving review is required to merge this pull request.

Assignees

@jopemachine jopemachine

Labels

size:XL 500~ LoC

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jopemachine