From 15bd7df96ef6f067f3b62ef675650c73deffcd68 Mon Sep 17 00:00:00 2001 From: prode Date: Sun, 20 Sep 2026 18:00:23 -0300 Subject: [PATCH 1/2] fix(interface): collect the UI panel that was paid for, and read the library back `POST /create-ui-asset` completes into `GET /v2/ui-assets/{ui_asset_id}`, whose payload carries `image_url` and no bytes anywhere. `_decode_images` reads `image` and `images`, so it found nothing: a panel charged at thirty generations wrote its manifest, exited 0, and left no file. Reproduced live before the fix and verified after it, on a panel already on the account. - A completed job that yields no bytes is fetched from the address it gives, on the waited-for path and on `job show`, which is what a timed-out call is told to run. - `ui` becomes a group: `ui new` generates as `ui` did, `ui list` names every panel the account holds, and `ui show ` writes one out again. Both readers are free, so a lost file is no longer thirty generations. - `approve` no longer gates a call whose estimate reports a price of zero. R6.1 asks agreement for a call that can cost money, and `character sheet` was asking for --yes to download a file. An absent estimate still gates. From the security review, both before merge: - A path parameter carrying a separator or a dot segment is refused in `_split_path`, for every route rather than at the third call site to remember it: the request that carries it carries the bearer token. - A download named by a provider response is refused past a ceiling far above any asset this tool makes. The address is fetched with nobody asking, so how much is read is not the provider's decision. specs/provider-core R4.9, R3.6, R3.7; specs/interface-portrait-and-font R1.5, R1.6, R1.7; specs/asset-workspace R6.5. Verified with scc validate --checks --pr: 1723 passed, 58 skipped, coverage 95.28%, ruff check and format clean, artifacts clean. --- .claude/skills/pixellab-cli-assets/SKILL.md | 2 +- .../skills/pixellab-cli-interface/SKILL.md | 30 ++- README.md | 4 +- docs/notes.md | 3 + docs/wiki/pages/ui-assets.md | 2 +- specs/asset-workspace/design.md | 14 ++ specs/asset-workspace/requirements.md | 1 + specs/asset-workspace/tasks.md | 4 + specs/interface-portrait-and-font/design.md | 25 ++- .../requirements.md | 3 + specs/interface-portrait-and-font/tasks.md | 14 ++ specs/provider-core/design.md | 41 ++++ specs/provider-core/requirements.md | 3 + specs/provider-core/tasks.md | 9 + src/pixellab_cli/catalog.py | 24 +++ src/pixellab_cli/commands/interface.py | 118 +++++++++++- src/pixellab_cli/pixellab.py | 78 +++++++- src/pixellab_cli/run.py | 12 ++ .../skill/pixellab-cli-assets/SKILL.md | 2 +- .../skill/pixellab-cli-interface/SKILL.md | 30 ++- tests/test_commands_interface.py | 178 ++++++++++++++++-- tests/test_pixellab.py | 146 +++++++++++++- tests/test_run.py | 48 +++++ 23 files changed, 749 insertions(+), 42 deletions(-) diff --git a/.claude/skills/pixellab-cli-assets/SKILL.md b/.claude/skills/pixellab-cli-assets/SKILL.md index 84c6973..7e99a43 100644 --- a/.claude/skills/pixellab-cli-assets/SKILL.md +++ b/.claude/skills/pixellab-cli-assets/SKILL.md @@ -103,7 +103,7 @@ file rather than the command. | A pose or state, any animation, a walk cycle, an attack, interpolation | `pixellab-cli-animation` | `pixellab-cli character state` | | A change to art that exists, a masked redraw, cleanup | `pixellab-cli-editing` | `pixellab-cli edit` · `pixellab-cli inpaint` | | Ground, terrain, tiles, props, objects | `pixellab-cli-scenes` | `pixellab-cli tiles` · `pixellab-cli object` | -| A UI panel, a pixel font | `pixellab-cli-interface` | `pixellab-cli ui` · `pixellab-cli font` | +| A UI panel, a pixel font | `pixellab-cli-interface` | `pixellab-cli ui new` · `pixellab-cli font` | Everything below is this skill's own: the money, the credentials, where files land, recovering a stopped run, and the free local tooling every category uses. diff --git a/.claude/skills/pixellab-cli-interface/SKILL.md b/.claude/skills/pixellab-cli-interface/SKILL.md index f5698a2..f9022c4 100644 --- a/.claude/skills/pixellab-cli-interface/SKILL.md +++ b/.claude/skills/pixellab-cli-interface/SKILL.md @@ -1,6 +1,6 @@ --- name: pixellab-cli-interface -description: Make the parts of a game a player reads — a UI panel with `pixellab-cli ui` and a pixel font with a usable TTF via `pixellab-cli font`. Use it for a dialogue box, an inventory frame, a HUD, a health bar, buttons, menus, a title screen's lettering, or any interface asset. +description: Make the parts of a game a player reads — a UI panel with `pixellab-cli ui new` and a pixel font with a usable TTF via `pixellab-cli font`. Use it for a dialogue box, an inventory frame, a HUD, a health bar, buttons, menus, a title screen's lettering, or any interface asset. --- What a player reads rather than what they move. @@ -10,8 +10,10 @@ Read `pixellab-cli-assets` first for the spending rule and the credentials. ## A panel ``` -pixellab-cli ui --size --element --piece --concept --route - --palette --style --name --seed +pixellab-cli ui new --size --element --piece --concept --route + --palette --style --name --seed +pixellab-cli ui list what the account holds. Free. +pixellab-cli ui show write one panel to the workspace. Free. ``` **Pro pricing**, twenty to forty generations. `--size` is 192 to 688 per axis, and the @@ -23,7 +25,7 @@ no coordinates: `button`, `icon_button`, `toolbar`, `tab`, `panel`, `window`, `health_bar`, `avatar`, and the polygons `triangle`, `pentagon`, `hexagon`, `octagon`. ```bash -pixellab-cli ui "deep charcoal slate framed by cool iron, hearth-fire orange accents" \ +pixellab-cli ui new "deep charcoal slate framed by cool iron, hearth-fire orange accents" \ --element panel --element health_bar --element button --element button ``` @@ -36,7 +38,7 @@ takes `x, y, r`, `polygon` takes `x, y, r, sides, phase`, and each needs a uniqu and a `kind`. The shapes are validated server side. ```bash -pixellab-cli ui "wooden RPG panel with gold trim" \ +pixellab-cli ui new "wooden RPG panel with gold trim" \ --piece '{"id":"frame","kind":"rounded_rect","x":0,"y":0,"w":512,"h":320,"radius":12}' \ --piece '{"id":"bar","kind":"rounded_rect","x":24,"y":24,"w":200,"h":20,"radius":6}' ``` @@ -64,8 +66,8 @@ a 48-pixel icon button, a 64-pixel HUD frame were previously impossible at any p because nothing else on this surface made UI. ```bash -pixellab-cli ui "a rusted iron inventory slot" --size 48 -pixellab-cli ui "a health bar in this style" --concept mockup.png +pixellab-cli ui new "a rusted iron inventory slot" --size 48 +pixellab-cli ui new "a health bar in this style" --concept mockup.png ``` `--concept` steers *what the thing is*; `--style` steers *what it looks like*, and it @@ -114,3 +116,17 @@ an engine and in a document. `--glyph-px` is the glyph size: 8, 16, 32 or 64. Pick it from where the text will be read — 8 is a HUD label, 32 and 64 are a title screen. + + +## The account keeps every panel + +A panel generated here is saved under the account, so the file on disk is a copy rather +than the only one. `ui list` names what is there — identifier, name, size, status and +the description it was made from — and `ui show ` writes one out again. +Both are free: the panel was paid for when it was made. + +So a lost file is not thirty generations. Reach for `ui list` before generating a panel +that sounds like one already made, and `ui show` rather than `ui new` to get it back. + +A panel still being generated reports how far along it is and writes nothing; run +`ui show` again when it is done. diff --git a/README.md b/README.md index d7ab794..4b61fd9 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,9 @@ pixellab-cli tiles terrain --lower grass --upper stone pixellab-cli tiles platform --material "stone bricks" pixellab-cli edit knight.png -p "give him a blue cape" pixellab-cli inpaint knight.png --mask mask.png -p "a horned helmet" -pixellab-cli ui "wooden RPG panel with gold trim" +pixellab-cli ui new "wooden RPG panel with gold trim" +pixellab-cli ui list # every panel the account holds. Free. +pixellab-cli ui show # write one out again. Free. pixellab-cli font "warm orange arcade font" --bold pixellab-cli art boxart "a knight at dawn over a burning keep" pixellab-cli image flip walk-east-*.png diff --git a/docs/notes.md b/docs/notes.md index f7d198d..67c23ef 100644 --- a/docs/notes.md +++ b/docs/notes.md @@ -93,3 +93,6 @@ over this file answers with the example above as well as with the notes. --> - n-0053 2026-09-20 #gotcha @src/pixellab_cli/commands/character.py — characters/animations names a v3 animation custom- plus the action's first thirty characters, truncated mid-word, which is how a motion is matched to one the character already has - n-0054 2026-09-20 #gotcha @src/pixellab_cli/commands/character.py — animation_group_id is extra_forbidden on characters/animations and only the site's unprefixed animate-with-text-v3/character/background takes it, which refuses the API key - n-0055 2026-09-20 #ceiling @src/pixellab_cli/subject.py — a motion is matched to one the character already has by the name the call would carry, so a bare action on one call and --name on the next are two motions and neither the refusal nor the join reporting fires +- n-0056 2026-09-20 #gotcha @src/pixellab_cli/pixellab.py — ui-assets/{id} returns image_url and no base64, so a completed job with no bytes is fetched from its address in _await rather than in _decode_images +- n-0057 2026-09-20 #gotcha @src/pixellab_cli/run.py — approve gates on the estimate, not the route, so a reported zero passes and an absent estimate still asks +- n-0058 2026-09-20 #gotcha @src/pixellab_cli/pixellab.py — a path parameter is refused in _split_path when it holds a separator or a dot segment, so no route has to remember the check diff --git a/docs/wiki/pages/ui-assets.md b/docs/wiki/pages/ui-assets.md index 6b9adba..8876fe6 100644 --- a/docs/wiki/pages/ui-assets.md +++ b/docs/wiki/pages/ui-assets.md @@ -12,7 +12,7 @@ Both take a `color_palette` **in words** — `"brown and gold"` — where most o takes a palette as a `color_image`. Worth knowing before encoding a PNG nobody wanted. That is about the palette only; both routes do take an image for guidance. -`create-ui-asset` has a `style_image`, which `pixellab-cli ui --style` already sends, and +`create-ui-asset` has a `style_image`, which `pixellab-cli ui new --style` already sends, and `generate-ui-v2` has a `concept_image` for steering a single element's design. ## The panel route describes a shape, not just a subject diff --git a/specs/asset-workspace/design.md b/specs/asset-workspace/design.md index b5c8fd2..49ca04f 100644 --- a/specs/asset-workspace/design.md +++ b/specs/asset-workspace/design.md @@ -109,3 +109,17 @@ of one character are gathered by the motion they animate, which is the name the carried or the action it was made from, and the gathered animation holds every direction they covered and every file they wrote. An atlas built from one animation is then built from every direction of it, which is what `export atlas` is given. + +## Agreement is for a call that can cost something + +`approve` gated every call that reached the runner, including the ones whose price is +known to be zero — reading a spritesheet, or writing out a UI panel the account already +holds. R6.1 asks for agreement on *a call that can cost money*, so gating a free +download was never what it said; it made `character sheet` ask for `--yes` to fetch a +file, and the tests never saw it because the suite sets the assume-yes variable for +every run. + +A free route is one whose estimate is present, reports zero, and is not a guess — +`Cost(generations=0.0, source="reported")`, which is how the library routes already +declare themselves. An absent estimate still gates: not knowing a price is not the same +as knowing it is nothing, and it is the case the gate exists for. diff --git a/specs/asset-workspace/requirements.md b/specs/asset-workspace/requirements.md index cbfc04f..0f37245 100644 --- a/specs/asset-workspace/requirements.md +++ b/specs/asset-workspace/requirements.md @@ -61,6 +61,7 @@ A file nobody can account for is worth less than the same file with a manifest. - **R6.2** Where the person's agreement has not been stated, the asset workspace shall send no request and shall write no ledger entry, because neither can have happened. - **R6.3** Where there is nobody present to state the agreement, the asset workspace shall take it from a named environment variable instead. - **R6.4** While a call is a dry run, the asset workspace shall require no agreement, because nothing is sent. +- **R6.5** (ADDED) Where a route is known to cost nothing, the asset workspace shall require no agreement, because no charge can begin. ## R7 · The subject as an entity diff --git a/specs/asset-workspace/tasks.md b/specs/asset-workspace/tasks.md index 7ff5b84..98d0a40 100644 --- a/specs/asset-workspace/tasks.md +++ b/specs/asset-workspace/tasks.md @@ -70,3 +70,7 @@ - [x] 7.1 (Unit) Gather one character's animation runs for a motion into one — R7.5 _Reason the provider starts a new animation per call_ +## 8 · Agreement is for a call that can cost something + +- [x] 8.1 (Unit) Ask for no agreement where the route is known to cost nothing — R6.5 + _Reason a free download asked for --yes, and the suite never saw it_ diff --git a/specs/interface-portrait-and-font/design.md b/specs/interface-portrait-and-font/design.md index 25eb3db..f74fd60 100644 --- a/specs/interface-portrait-and-font/design.md +++ b/specs/interface-portrait-and-font/design.md @@ -10,7 +10,7 @@ ci: wait One module, `commands/interface.py`, three commands: ``` -pixellab-cli ui "wooden RPG panel with gold trim" create-ui-asset (Pro) R1.1 +pixellab-cli ui new "wooden RPG panel with gold trim" create-ui-asset (Pro) R1.1 pixellab-cli font "warm orange arcade font" --bold generate-font-pro (25) R2.1 pixellab-cli portrait knight.png portrait-character-pro (Pro) R3.1 ``` @@ -56,8 +56,7 @@ route's floor and inside `generate-ui-v2`'s, which starts at 16. ## Choosing between them -`pixellab-cli ui` keeps making a panel, so nothing that works today changes. It moves to -the single-element route on either of the two signals that mean a panel was never +`pixellab-cli ui new` makes a panel by default. It moves to the single-element route on either of the two signals that mean a panel was never possible or never intended (R1.3): | Given | Route | @@ -82,3 +81,23 @@ they wrote. The last two are the ones worth writing down, because neither fails: without the refusal the argument is simply absent from the body that gets sent. + +## The panel you already paid for + +`POST /create-ui-asset` saves what it makes: the account keeps every panel, and +`GET /v2/ui-assets` and `GET /v2/ui-assets/{ui_asset_id}` read them back, both free. +Nothing here called either, so a panel whose file was lost was a panel to buy again at +thirty generations — and until the fix in `specs/provider-core/` there was no file to +lose, because the address the job completes with was never fetched. + +`ui` becomes a group to make room for them, the same shape `character` and `object` +already have: `ui new ""` generates, `ui list` names what the account +holds, and `ui show ` reports one and writes its image. That renames the generating +form — `ui ""` was the whole command before — which the skill and the +README are updated for in the same change. + +`ui show` writes through the same runner every other command writes through, at no cost: +the download is a public CDN address, like a rotation URL, and the run is recorded with +the panel's identifier so the file can be traced back to the call that paid for it. A +panel still processing reports its progress and writes nothing, because `image_url` is +null until it is done. diff --git a/specs/interface-portrait-and-font/requirements.md b/specs/interface-portrait-and-font/requirements.md index eacf7df..fa79ec7 100644 --- a/specs/interface-portrait-and-font/requirements.md +++ b/specs/interface-portrait-and-font/requirements.md @@ -21,6 +21,9 @@ a feature of its own. - **R1.2** The interface commands shall say that a panel is priced as a Pro Tools route before calling it. - **R1.3** (ADDED) Where a concept image is given, or a size smaller than the panel route accepts, the interface commands shall generate one interface element rather than a panel, and shall say that it is priced as a Pro Tools route. - **R1.4** (ADDED) The interface commands shall accept a concept image to steer the design of one element. +- **R1.5** (ADDED) The interface commands shall list the UI panels the account holds, reporting for each its identifier, its name, the description it was made from, its size and its status. +- **R1.6** (ADDED) When asked to show one UI panel, the interface commands shall report it and shall write its image to the workspace, without calling a paid route. +- **R1.7** (ADDED) While a UI panel is still being generated, the interface commands shall report that and shall write no image for it. ## R2 · Fonts diff --git a/specs/interface-portrait-and-font/tasks.md b/specs/interface-portrait-and-font/tasks.md index ff3aca0..2a881bb 100644 --- a/specs/interface-portrait-and-font/tasks.md +++ b/specs/interface-portrait-and-font/tasks.md @@ -24,3 +24,17 @@ - [x] 4.1 (Unit) Report the route and the arguments under a dry run, sending nothing — R4.1 _Depends 1.1_ + +## 7 · The panel you already paid for + +- [x] 7.1 (Unit) Add the UI asset list and detail routes to the catalogue — R1.5, R1.6 + _Reason the UI library routes were never reached and a lost panel meant paying again_ +- [x] 7.2 (Unit) Make ui a group with ui new, and list the account's panels — R1.5 + _Depends 7.1_ + _Reason the UI library routes were never reached and a lost panel meant paying again_ +- [x] 7.3 (Unit) Show one panel, write it, and say when it is not ready — R1.6, R1.7 + _Depends 7.1_ + _Reason the UI library routes were never reached and a lost panel meant paying again_ +- [x] 7.4 (Unit) Teach the skill and the README the ui group — R1.5, R1.6 + _Depends 7.3_ + _Reason the UI library routes were never reached and a lost panel meant paying again_ diff --git a/specs/provider-core/design.md b/specs/provider-core/design.md index 6ad3f3a..750f167 100644 --- a/specs/provider-core/design.md +++ b/specs/provider-core/design.md @@ -140,3 +140,44 @@ raise site added later would not remember. the CDN, call `subscribe`, download the returned URLs to disk. The tool's own estimate stands in for the usage fal does not report (R5.2). Its errors are translated into the same hierarchy, so a caller handles one set. + +## An image that arrives as an address + +`_decode_images` reads `image` and `images`, which is every base64 field the routes +here return — and `POST /create-ui-asset` returns neither. Its job completes into +`GET /v2/ui-assets/{ui_asset_id}`, whose payload carries `image_url`, a public CDN +address, and no bytes at all: + +```json +{"id": "43d70268-…", "status": "completed", "size": {"width": 192, "height": 192}, + "image_url": "https://backblaze.pixellab.ai/file/…/full.png?v=1789936808"} +``` + +So a panel charged at thirty generations completed, wrote its manifest, and wrote no +image — the command exited `0` and the asset was never collected. The tests did not +catch it because they replied to the poll with `{"status": "completed", "images": […]}`, +a shape that route has never returned. + +The fix is where the job is awaited rather than in the decoder: fetching needs the +client, and `_decode_images` is a function over a payload. When a completed job yields +no bytes, `image_url` is downloaded through the same unauthenticated `download` the +rotation URLs already go through. Only `image_url`, and only because a real payload was +seen holding it — the same rule the decoder states for `quantized_images`. + +## What a value reaching the address may be + +A path parameter is interpolated into the route's path, and the request that carries it +carries the bearer token — so a separator or a dot segment in one is a URL of somebody +else's choosing, reached with the caller's credential. `collect` already refused it for +a job id and `character state` for a pose, each at its own call site; a third caller +with the same shape is the point at which the check belongs at the one place every path +parameter passes through instead. `_split_path` refuses it for every route, present and +future, before anything is sent (R3.6). + +The address a completed job hands back is the provider's, and R4.9 now fetches it with +nobody asking. How much of it is read is therefore not the provider's decision either: +`_fetch` refuses a body past a ceiling far above any asset this tool makes, by the +declared length where there is one and by what arrived where there is not (R3.7). +Nothing narrower than a size limit is attempted — PixelLab serves assets from more than +one host and a host list would fail closed on the day they add another, which for a +tool whose whole job is to collect what was paid for is the more expensive failure. diff --git a/specs/provider-core/requirements.md b/specs/provider-core/requirements.md index f671f2f..0ad976d 100644 --- a/specs/provider-core/requirements.md +++ b/specs/provider-core/requirements.md @@ -47,6 +47,8 @@ looks like have to be decided once. - **R3.3** If a caller supplies a parameter the route does not accept, a value outside an enumerated parameter's set, or a size the route rejects, then the provider core shall fail before the request is sent, naming the parameter and the values or bounds that are allowed. - **R3.4** The route table shall be derived from the vendored provider schemas under `reference/`, and a route whose shape contradicts those schemas shall be reported by the suite rather than at call time. - **R3.5** (ADDED) Where a route requires an image to match one of its size parameters, if the image given does not match it, then the provider core shall fail before the request is sent, naming both sizes. +- **R3.6** (ADDED) If a value that becomes part of a route's address carries a path separator or a dot segment, then the provider core shall refuse the call before anything is sent, because the request that would carry it carries the account's credential. +- **R3.7** (ADDED) If a download named by a provider's response is larger than the ceiling this tool holds for a generated asset, then the provider core shall refuse it rather than read it. ## R4 · Background jobs @@ -58,6 +60,7 @@ looks like have to be decided once. - **R4.6** (MODIFIED) Where a collected job is written, the provider core shall record no second call, and shall settle the call the job belongs to with what the job reports it cost. - **R4.8** (ADDED) If polling stops before a job resolves, then the provider core shall record that call as still running and shall record no cost for it, because a call that has not finished has not reported one. - **R4.7** (MODIFIED) Where a response declares how many images it holds and fewer were found, the provider core shall take them from a field it has seen carry them, and shall leave the count alone otherwise. +- **R4.9** (ADDED) Where a completed job reports the address of the image it made rather than its bytes, the provider core shall fetch that address and return the image, because the call has been charged and its result is otherwise never collected. ## R5 · Results and cost diff --git a/specs/provider-core/tasks.md b/specs/provider-core/tasks.md index 4025714..98c1ea4 100644 --- a/specs/provider-core/tasks.md +++ b/specs/provider-core/tasks.md @@ -62,3 +62,12 @@ _Depends 1.2_ - [x] 4.2 (Unit) Translate fal failures into the shared exception hierarchy, and return the tool's own estimate marked as an estimate — R2.5, R5.2 _Depends 4.1_ + +## 7 · An image that arrives as an address +- [x] 7.1 (TDD) Download a completed job's image_url when it returns no bytes — R4.9 + _Reason a UI panel charged at 30 generations wrote no image_ +- [x] 7.2 (TDD) Refuse a path parameter carrying a separator or a dot segment — R3.6 + _Reason security review found ui show sending the id unvalidated_ +- [x] 7.3 (TDD) Refuse a download past the ceiling for a generated asset — R3.7 + _Reason the address is now fetched unasked_ + diff --git a/src/pixellab_cli/catalog.py b/src/pixellab_cli/catalog.py index 820de5d..1441865 100644 --- a/src/pixellab_cli/catalog.py +++ b/src/pixellab_cli/catalog.py @@ -1166,6 +1166,28 @@ def _image( ) +LIST_UI_ASSETS = Route( + name="ui-assets", + method="GET", + path="/ui-assets", + kind=RouteKind.SYNCHRONOUS, + summary="Every UI panel on the account, newest first.", + estimated_generations=0.0, + params=(), +) + +GET_UI_ASSET = Route( + name="ui-asset", + method="GET", + path="/ui-assets/{ui_asset_id}", + kind=RouteKind.SYNCHRONOUS, + summary="One UI panel: its address, its size, and how it was made.", + estimated_generations=0.0, + params=(Param("ui_asset_id", ParamKind.STRING, required=True),), + path_params=("ui_asset_id",), +) + + ROUTES: tuple[Route, ...] = ( CREATE_IMAGE_PIXFLUX, CREATE_IMAGE_PIXEN, @@ -1208,6 +1230,8 @@ def _image( CHARACTER_SPRITESHEET, LIST_OBJECTS, GET_OBJECT, + LIST_UI_ASSETS, + GET_UI_ASSET, ) BY_NAME: dict[str, Route] = {route.name: route for route in ROUTES} diff --git a/src/pixellab_cli/commands/interface.py b/src/pixellab_cli/commands/interface.py index 34648b0..c18a883 100644 --- a/src/pixellab_cli/commands/interface.py +++ b/src/pixellab_cli/commands/interface.py @@ -35,8 +35,17 @@ GLYPH_SIZES = (8, 16, 32, 64) +# `ui` is a group rather than one command because the account keeps every panel it +# generates: `character` and `object` have the same shape for the same reason. The +# generating form moved to `ui new` when the library arrived. +ui_app = typer.Typer(name="ui", help="UI panels: generate one, or read what the account holds.") + + def register(app: typer.Typer) -> None: - app.command("ui")(ui) + ui_app.command("new")(ui) + ui_app.command("list")(list_panels) + ui_app.command("show")(show_panel) + app.add_typer(ui_app) app.command("font")(font) app.command("portrait")(portrait) @@ -435,3 +444,110 @@ def _sized(path: Path) -> dict[str, Any]: "image": encoded.as_payload(), "size": {"width": encoded.width, "height": encoded.height}, } + + +def _panel_line(asset: dict[str, Any]) -> str: + """One panel, as a person reads it: the id first, because it is what gets copied.""" + size = asset.get("size") or {} + measured = f"{size.get('width', '?')}x{size.get('height', '?')}" + name = asset.get("name") or "unnamed" + status = asset.get("status") or "unknown" + return ( + f"{asset.get('id', '?')} {name} {measured} {status} {asset.get('prompt', '')}".rstrip() + ) + + +def list_panels(context: typer.Context) -> None: + """Every UI panel on the account, newest first. Free.""" + try: + _list_panels(context) + except PixellabCliError as failure: + output.handle(failure) + + +def _list_panels(context: typer.Context) -> None: + app_context: AppContext = context.obj + payload = app_context.pixellab().call("ui-assets").raw + assets = [asset for asset in payload.get("ui_assets") or [] if isinstance(asset, dict)] + if not assets: + output.emit(payload, ["no UI panels on this account"], as_json=app_context.as_json) + return + + total = payload.get("total") + lines = [_panel_line(asset) for asset in assets] + # The route answers 50 at a time and says how many there are. Paging is not worth + # a flag yet; saying the listing is partial is worth a line. + if isinstance(total, int) and total > len(assets): + lines.append(f"{len(assets)} of {total}, newest first") + output.emit(payload, lines, as_json=app_context.as_json) + + +def show_panel( + context: typer.Context, + ui_asset_id: str = typer.Argument(..., help="The panel to read."), + name: str = typer.Option(None, "--name", help="What to call the file."), +) -> None: + """One UI panel, written to the workspace. Free: it was paid for when it was made.""" + try: + _show_panel(context, ui_asset_id, name) + except PixellabCliError as failure: + output.handle(failure) + + +def _show_panel(context: typer.Context, ui_asset_id: str, name: str | None) -> None: + app_context: AppContext = context.obj + route = catalog.route("ui-asset") + estimate = Cost(generations=0.0, source="reported") + + if app_context.dry_run: + body = build_request(route, {"ui_asset_id": ui_asset_id}) + output.emit( + output.dry_run_payload("pixellab", route.name, body, estimate), + output.describe_dry_run("pixellab", route.name, estimate), + as_json=app_context.as_json, + ) + return + + client = app_context.pixellab() + payload = client.call(route.name, ui_asset_id=ui_asset_id).raw + if not payload: + raise ValidationError(f"no UI panel {ui_asset_id!r} on this account") + + address = payload.get("image_url") + if not isinstance(address, str) or not address: + # `image_url` is null until the panel is done, and the payload says how far + # along it is. Reported rather than refused: nothing is wrong, it is early. + progress = payload.get("progress_percent") + eta = payload.get("eta_seconds") + lines = [ + f"{_panel_line(payload)}", + "no image yet" + + (f", {progress}% done" if progress is not None else "") + + (f", about {eta}s left" if eta is not None else ""), + ] + output.emit(payload, lines, as_json=app_context.as_json) + return + + outcome = app_context.runner.run( + subject=app_context.subject, + kind="interface", + description=f"{ui_asset_id} panel", + provider="pixellab", + route=route.name, + arguments={"ui_asset_id": ui_asset_id}, + call=lambda: Result( + route=route.name, + images=[client.download(address)], + raw=payload, + ids={"ui_asset_id": ui_asset_id}, + ), + translate=from_pixellab, + estimate=estimate, + name=name or payload.get("name") or "ui-panel", + links={"ui_asset_id": ui_asset_id}, + ) + output.emit( + output.run_payload(outcome), + [_panel_line(payload), *output.describe_run(outcome)], + as_json=app_context.as_json, + ) diff --git a/src/pixellab_cli/pixellab.py b/src/pixellab_cli/pixellab.py index 2666473..1edf33c 100644 --- a/src/pixellab_cli/pixellab.py +++ b/src/pixellab_cli/pixellab.py @@ -31,6 +31,10 @@ # A synchronous generation route holds the connection open while it generates, so # its timeout is measured in minutes; everything else should answer at once. REQUEST_TIMEOUT = 30.0 +# How much of a provider-named URL is read before it is refused. Well above any +# asset this tool makes, and well below what would hurt to hold in memory. +MAX_DOWNLOAD_BYTES = 64 * 1024 * 1024 + GENERATION_TIMEOUT = 300.0 DEFAULT_MAX_ATTEMPTS = 4 @@ -144,7 +148,7 @@ def _fetch(self, url: str, *, authenticated: bool = True) -> bytes: with httpx.Client(timeout=GENERATION_TIMEOUT) as client: response = client.get(url, headers=headers) response.raise_for_status() - return response.content + return self._within_the_ceiling(response, url) except httpx.HTTPError as failure: raise ProviderError( f"could not download from PixelLab: {failure}", @@ -152,6 +156,36 @@ def _fetch(self, url: str, *, authenticated: bool = True) -> bytes: secrets=self._credentials.secrets, ) from failure + def _within_the_ceiling(self, response: httpx.Response, url: str) -> bytes: + """The body, refused rather than held where it is larger than any asset here. + + The address comes from the provider's response and is fetched without anybody + asking, so how much is read must not be the provider's decision. The ceiling + is far above every asset this tool generates — a spritesheet ZIP of eight + directions is orders below it — so reaching it means something is wrong + rather than something is large. + """ + declared = response.headers.get("Content-Length") + try: + if declared is not None and int(declared) > MAX_DOWNLOAD_BYTES: + raise ProviderError( + f"refused a {int(declared)} byte download: nothing here is that " + f"large, and the ceiling is {MAX_DOWNLOAD_BYTES}", + context={"url": url, "bytes": int(declared)}, + secrets=self._credentials.secrets, + ) + except ValueError: + pass + body = response.content + if len(body) > MAX_DOWNLOAD_BYTES: + raise ProviderError( + f"refused a {len(body)} byte download: nothing here is that large, " + f"and the ceiling is {MAX_DOWNLOAD_BYTES}", + context={"url": url, "bytes": len(body)}, + secrets=self._credentials.secrets, + ) + return body + # ------------------------------------------------------------------ requests def _request( @@ -251,7 +285,16 @@ def _await(self, route: Route, result: Result) -> Result: secrets=self._credentials.secrets, ) payload = self._poll(job_id, route.poll_path or "", route.name) - return _complete(route, payload, result) + result = _complete(route, payload, result) + if not result.images: + # `create-ui-asset` completes into `/ui-assets/{id}`, which carries the + # panel's address and no bytes anywhere — so the decoder found nothing and + # a call charged at thirty generations wrote no image. Fetched here rather + # than in the decoder, which is a function over a payload and has no + # client. Only `image_url`, and only because a real payload was seen + # holding it: the same rule the decoder states for `quantized_images`. + result.images = [self.download(address) for address in _image_urls(payload)] + return result def collect(self, job_id: str) -> Result: """Wait for a background job named by its id alone, and return what it made. @@ -275,7 +318,13 @@ def collect(self, job_id: str) -> Result: payload = self._poll(job_id, catalog.BACKGROUND_JOBS_PATH, f"job {job_id}") body = payload.get("last_response") or payload result = Result(route="background-job", job_id=job_id, raw=payload) - result.images = _decode_images(body) + result.images = _decode_images(body) or [ + # The same address the waited-for path fetches. This is the path a + # `create-ui-asset` that outran the wait is *told* to use, so a panel + # already charged for must not be lost one hop later. + self.download(address) + for address in _image_urls(body) + ] result.usage = _usage(payload) or _usage(body) or Usage(estimated=True) # The same provenance the ordinary path keeps. A collected job knowing less # about itself than a waited-for one is drift, not a decision. @@ -343,6 +392,13 @@ def _complete(route: Route, payload: dict[str, Any], result: Result) -> Result: return result +def _image_urls(payload: dict[str, Any]) -> list[str]: + """The addresses a completed job gives instead of bytes, if it gives any.""" + body = payload.get("last_response") or payload + address = body.get("image_url") + return [address] if isinstance(address, str) and address else [] + + def _decode_images(payload: dict[str, Any]) -> list[bytes]: """Every image in a response, whether it arrived as `image` or as `images`. @@ -398,12 +454,28 @@ def _poll_id(route: Route, payload: dict[str, Any]) -> str | None: return None +# What may not appear in a value that becomes part of a URL path. A separator opens a +# segment of somebody else's choosing and a dot segment climbs out of the one this +# route owns — either way the request still carries the bearer token, so the check is +# here, at the one place every path parameter passes through, rather than at each +# caller that remembers to make it. +PATH_SEPARATORS = ("/", "\\") + + def _split_path(route: Route, body: dict[str, Any]) -> tuple[str, dict[str, Any]]: """Move the path parameters out of the body and into the URL.""" if not route.path_params: return route.path, body remaining = dict(body) values = {name: remaining.pop(name, "") for name in route.path_params} + for name, value in values.items(): + text = str(value) + if any(mark in text for mark in PATH_SEPARATORS) or ".." in text: + raise ValidationError( + f"{name} is part of the address this is sent to, so it carries no " + f"path of its own: {value!r}", + context={"route": route.name, name: value}, + ) return route.path.format(**values), remaining diff --git a/src/pixellab_cli/run.py b/src/pixellab_cli/run.py index 0c9c0f0..3a99c7b 100644 --- a/src/pixellab_cli/run.py +++ b/src/pixellab_cli/run.py @@ -27,6 +27,12 @@ MANIFEST_SCHEMA = 1 + +def _costs_something(estimate: Cost) -> bool: + """Whether an estimate claims any price at all, in any of the units it carries.""" + return bool(estimate.generations or estimate.usd or estimate.seconds) + + # What stands in for `--yes` where there is nobody to type it: a headless run, a CI # job, a script the person wrote themselves. Deliberately an environment variable and # not a setting in the credentials file — it is turned on once, outside the run, by @@ -187,6 +193,12 @@ def approve( """ if self.approved or _assumed_yes(): return + # A route whose price is known to be zero is not a call that can cost money, + # which is all R6.1 asks agreement for. Known is the load-bearing word: an + # absent estimate still gates, because not knowing a price is not the same as + # knowing it is nothing, and that is the case the gate exists for. + if estimate is not None and estimate.source == REPORTED and not _costs_something(estimate): + return generations = estimate.generations if estimate else 0.0 cost = ( f"about {generations:g} generation{'' if generations == 1 else 's'}" diff --git a/src/pixellab_cli/skill/pixellab-cli-assets/SKILL.md b/src/pixellab_cli/skill/pixellab-cli-assets/SKILL.md index 84c6973..7e99a43 100644 --- a/src/pixellab_cli/skill/pixellab-cli-assets/SKILL.md +++ b/src/pixellab_cli/skill/pixellab-cli-assets/SKILL.md @@ -103,7 +103,7 @@ file rather than the command. | A pose or state, any animation, a walk cycle, an attack, interpolation | `pixellab-cli-animation` | `pixellab-cli character state` | | A change to art that exists, a masked redraw, cleanup | `pixellab-cli-editing` | `pixellab-cli edit` · `pixellab-cli inpaint` | | Ground, terrain, tiles, props, objects | `pixellab-cli-scenes` | `pixellab-cli tiles` · `pixellab-cli object` | -| A UI panel, a pixel font | `pixellab-cli-interface` | `pixellab-cli ui` · `pixellab-cli font` | +| A UI panel, a pixel font | `pixellab-cli-interface` | `pixellab-cli ui new` · `pixellab-cli font` | Everything below is this skill's own: the money, the credentials, where files land, recovering a stopped run, and the free local tooling every category uses. diff --git a/src/pixellab_cli/skill/pixellab-cli-interface/SKILL.md b/src/pixellab_cli/skill/pixellab-cli-interface/SKILL.md index f5698a2..f9022c4 100644 --- a/src/pixellab_cli/skill/pixellab-cli-interface/SKILL.md +++ b/src/pixellab_cli/skill/pixellab-cli-interface/SKILL.md @@ -1,6 +1,6 @@ --- name: pixellab-cli-interface -description: Make the parts of a game a player reads — a UI panel with `pixellab-cli ui` and a pixel font with a usable TTF via `pixellab-cli font`. Use it for a dialogue box, an inventory frame, a HUD, a health bar, buttons, menus, a title screen's lettering, or any interface asset. +description: Make the parts of a game a player reads — a UI panel with `pixellab-cli ui new` and a pixel font with a usable TTF via `pixellab-cli font`. Use it for a dialogue box, an inventory frame, a HUD, a health bar, buttons, menus, a title screen's lettering, or any interface asset. --- What a player reads rather than what they move. @@ -10,8 +10,10 @@ Read `pixellab-cli-assets` first for the spending rule and the credentials. ## A panel ``` -pixellab-cli ui --size --element --piece --concept --route - --palette --style --name --seed +pixellab-cli ui new --size --element --piece --concept --route + --palette --style --name --seed +pixellab-cli ui list what the account holds. Free. +pixellab-cli ui show write one panel to the workspace. Free. ``` **Pro pricing**, twenty to forty generations. `--size` is 192 to 688 per axis, and the @@ -23,7 +25,7 @@ no coordinates: `button`, `icon_button`, `toolbar`, `tab`, `panel`, `window`, `health_bar`, `avatar`, and the polygons `triangle`, `pentagon`, `hexagon`, `octagon`. ```bash -pixellab-cli ui "deep charcoal slate framed by cool iron, hearth-fire orange accents" \ +pixellab-cli ui new "deep charcoal slate framed by cool iron, hearth-fire orange accents" \ --element panel --element health_bar --element button --element button ``` @@ -36,7 +38,7 @@ takes `x, y, r`, `polygon` takes `x, y, r, sides, phase`, and each needs a uniqu and a `kind`. The shapes are validated server side. ```bash -pixellab-cli ui "wooden RPG panel with gold trim" \ +pixellab-cli ui new "wooden RPG panel with gold trim" \ --piece '{"id":"frame","kind":"rounded_rect","x":0,"y":0,"w":512,"h":320,"radius":12}' \ --piece '{"id":"bar","kind":"rounded_rect","x":24,"y":24,"w":200,"h":20,"radius":6}' ``` @@ -64,8 +66,8 @@ a 48-pixel icon button, a 64-pixel HUD frame were previously impossible at any p because nothing else on this surface made UI. ```bash -pixellab-cli ui "a rusted iron inventory slot" --size 48 -pixellab-cli ui "a health bar in this style" --concept mockup.png +pixellab-cli ui new "a rusted iron inventory slot" --size 48 +pixellab-cli ui new "a health bar in this style" --concept mockup.png ``` `--concept` steers *what the thing is*; `--style` steers *what it looks like*, and it @@ -114,3 +116,17 @@ an engine and in a document. `--glyph-px` is the glyph size: 8, 16, 32 or 64. Pick it from where the text will be read — 8 is a HUD label, 32 and 64 are a title screen. + + +## The account keeps every panel + +A panel generated here is saved under the account, so the file on disk is a copy rather +than the only one. `ui list` names what is there — identifier, name, size, status and +the description it was made from — and `ui show ` writes one out again. +Both are free: the panel was paid for when it was made. + +So a lost file is not thirty generations. Reach for `ui list` before generating a panel +that sounds like one already made, and `ui show` rather than `ui new` to get it back. + +A panel still being generated reports how far along it is and writes nothing; run +`ui show` again when it is done. diff --git a/tests/test_commands_interface.py b/tests/test_commands_interface.py index 3b9c37b..e6976af 100644 --- a/tests/test_commands_interface.py +++ b/tests/test_commands_interface.py @@ -46,7 +46,7 @@ def test_a_panel_is_generated_and_written(self, tmp_path, monkeypatch): json={"status": "completed", "images": [image_payload()]} ) - result = invoke(["ui", "wooden RPG panel"], tmp_path, monkeypatch) + result = invoke(["ui", "new", "wooden RPG panel"], tmp_path, monkeypatch) assert result.exit_code == 0 assert list((tmp_path / "out").glob("*/*.png")) @@ -60,7 +60,11 @@ def test_named_elements_reach_the_request(self, tmp_path, monkeypatch): json={"status": "completed", "images": [image_payload()]} ) - invoke(["ui", "a panel", "--element", "button", "--element", "tab"], tmp_path, monkeypatch) + invoke( + ["ui", "new", "a panel", "--element", "button", "--element", "tab"], + tmp_path, + monkeypatch, + ) assert json.loads(route.calls.last.request.content)["elements"] == ["button", "tab"] @@ -73,7 +77,7 @@ def test_the_price_is_announced(self, tmp_path, monkeypatch): json={"status": "completed", "images": [image_payload()]} ) - result = invoke(["ui", "a panel"], tmp_path, monkeypatch) + result = invoke(["ui", "new", "a panel"], tmp_path, monkeypatch) assert "generations" in result.output @@ -82,14 +86,14 @@ def test_a_size_below_the_panel_floor_is_no_longer_refused(self, tmp_path, monke # 64-pixel element could not be generated at all. It now reaches generate-ui-v2, # which is the point of adding that route. result = invoke( - ["--dry-run", "--json", "ui", "a button", "--size", "64"], tmp_path, monkeypatch + ["--dry-run", "--json", "ui", "new", "a button", "--size", "64"], tmp_path, monkeypatch ) assert result.exit_code == 0 assert json.loads(result.stdout)["route"] == "generate-ui-v2" def test_a_size_outside_both_routes_is_still_refused(self, tmp_path, monkeypatch): - result = invoke(["ui", "a panel", "--size", "900"], tmp_path, monkeypatch) + result = invoke(["ui", "new", "a panel", "--size", "900"], tmp_path, monkeypatch) assert result.exit_code == 2 @@ -237,7 +241,7 @@ def test_a_file_that_is_not_there_is_refused(self, tmp_path, monkeypatch): class TestDryRun: def test_nothing_is_sent(self, tmp_path, monkeypatch): - result = invoke(["--dry-run", "ui", "a panel"], tmp_path, monkeypatch) + result = invoke(["--dry-run", "ui", "new", "a panel"], tmp_path, monkeypatch) assert result.exit_code == 0 assert "create-ui-asset" in result.stdout @@ -255,7 +259,7 @@ class TestAnExplicitPanelShape: def _sent(self, tmp_path, monkeypatch, *extra): result = invoke( - ["--dry-run", "--json", "ui", "wooden RPG panel", *extra], tmp_path, monkeypatch + ["--dry-run", "--json", "ui", "new", "wooden RPG panel", *extra], tmp_path, monkeypatch ) return json.loads(result.stdout)["arguments"] @@ -280,7 +284,7 @@ def test_several_pieces_keep_their_order(self, tmp_path, monkeypatch): def test_a_piece_that_is_not_json_is_refused_by_name(self, tmp_path, monkeypatch): result = invoke( - ["--dry-run", "ui", "wooden RPG panel", "--piece", "a rounded rectangle"], + ["--dry-run", "ui", "new", "wooden RPG panel", "--piece", "a rounded rectangle"], tmp_path, monkeypatch, ) @@ -290,7 +294,7 @@ def test_a_piece_that_is_not_json_is_refused_by_name(self, tmp_path, monkeypatch def test_a_piece_that_is_json_but_not_an_object_is_refused(self, tmp_path, monkeypatch): result = invoke( - ["--dry-run", "ui", "wooden RPG panel", "--piece", "[1, 2, 3]"], + ["--dry-run", "ui", "new", "wooden RPG panel", "--piece", "[1, 2, 3]"], tmp_path, monkeypatch, ) @@ -316,7 +320,7 @@ def _concept(self, tmp_path, name="concept.png"): def _route(self, tmp_path, monkeypatch, *extra): result = invoke( - ["--dry-run", "--json", "ui", "a medieval stone button", *extra], + ["--dry-run", "--json", "ui", "new", "a medieval stone button", *extra], tmp_path, monkeypatch, ) @@ -341,7 +345,7 @@ def test_a_concept_image_makes_one_element(self, tmp_path, monkeypatch): def test_a_layout_below_the_floor_is_refused(self, tmp_path, monkeypatch): result = invoke( - ["--dry-run", "ui", "a panel", "--size", "48", "--element", "button"], + ["--dry-run", "ui", "new", "a panel", "--size", "48", "--element", "button"], tmp_path, monkeypatch, ) @@ -355,7 +359,16 @@ def test_the_route_can_be_named_explicitly(self, tmp_path, monkeypatch): def test_naming_the_element_route_with_a_layout_is_refused(self, tmp_path, monkeypatch): # Dropping --element silently is how a surprising image gets billed. result = invoke( - ["--dry-run", "ui", "a button", "--route", "generate-ui-v2", "--element", "button"], + [ + "--dry-run", + "ui", + "new", + "a button", + "--route", + "generate-ui-v2", + "--element", + "button", + ], tmp_path, monkeypatch, ) @@ -365,7 +378,7 @@ def test_naming_the_element_route_with_a_layout_is_refused(self, tmp_path, monke def test_an_unknown_route_is_refused_by_name(self, tmp_path, monkeypatch): result = invoke( - ["--dry-run", "ui", "a button", "--route", "nonsense"], tmp_path, monkeypatch + ["--dry-run", "ui", "new", "a button", "--route", "nonsense"], tmp_path, monkeypatch ) assert result.exit_code != 0 @@ -378,6 +391,7 @@ def test_a_concept_with_a_layout_is_refused(self, tmp_path, monkeypatch): [ "--dry-run", "ui", + "new", "a panel", "--concept", self._concept(tmp_path), @@ -393,7 +407,16 @@ def test_a_concept_with_a_layout_is_refused(self, tmp_path, monkeypatch): def test_a_style_image_on_the_element_route_is_refused(self, tmp_path, monkeypatch): result = invoke( - ["--dry-run", "ui", "a button", "--size", "48", "--style", self._concept(tmp_path)], + [ + "--dry-run", + "ui", + "new", + "a button", + "--size", + "48", + "--style", + self._concept(tmp_path), + ], tmp_path, monkeypatch, ) @@ -403,7 +426,7 @@ def test_a_style_image_on_the_element_route_is_refused(self, tmp_path, monkeypat def test_the_concept_image_is_sent_with_its_size(self, tmp_path, monkeypatch): result = invoke( - ["--dry-run", "--json", "ui", "a button", "--concept", self._concept(tmp_path)], + ["--dry-run", "--json", "ui", "new", "a button", "--concept", self._concept(tmp_path)], tmp_path, monkeypatch, ) @@ -420,6 +443,7 @@ def test_a_wide_element_the_schema_allows_is_accepted(self, tmp_path, monkeypatc "--dry-run", "--json", "ui", + "new", "a wide banner", "--route", "generate-ui-v2", @@ -438,9 +462,131 @@ def test_a_wide_element_the_schema_allows_is_accepted(self, tmp_path, monkeypatc def test_taller_than_the_element_route_allows_is_refused(self, tmp_path, monkeypatch): result = invoke( - ["--dry-run", "ui", "a tall banner", "--route", "generate-ui-v2", "--size", "300x750"], + [ + "--dry-run", + "ui", + "new", + "a tall banner", + "--route", + "generate-ui-v2", + "--size", + "300x750", + ], tmp_path, monkeypatch, ) assert result.exit_code != 0 + + +PANEL_URL = "https://backblaze.pixellab.ai/file/pixellab-characters/ui/x/y/full.png?v=1" + + +def a_panel(asset_id="ui-1", status="completed", image_url=PANEL_URL, **extra): + """The shape `/ui-assets` really returns, read off a live account.""" + return { + "id": asset_id, + "name": "probe-panel", + "prompt": "wooden RPG panel with gold trim", + "size": {"width": 192, "height": 192}, + "image_url": image_url, + "status": status, + "created_at": "2026-09-20T20:40:08.011521+00:00", + **extra, + } + + +class TestListingThePanelsTheAccountHolds: + """R1.5. Free, and the answer to 'what did I already pay for'.""" + + @respx.mock + def test_each_panel_is_named_with_what_it_takes_to_use_it(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/ui-assets").respond( + json={"ui_assets": [a_panel()], "total": 1} + ) + + result = invoke(["ui", "list"], tmp_path, monkeypatch) + + assert result.exit_code == 0 + assert "ui-1" in result.output + assert "probe-panel" in result.output + assert "192x192" in result.output + assert "completed" in result.output + + @respx.mock + def test_an_empty_account_says_so(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/ui-assets").respond(json={"ui_assets": [], "total": 0}) + + result = invoke(["ui", "list"], tmp_path, monkeypatch) + + assert "no UI panels" in result.output + + @respx.mock + def test_a_listing_the_route_cut_short_says_it_is_partial(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/ui-assets").respond( + json={"ui_assets": [a_panel(), a_panel("ui-2")], "total": 120} + ) + + result = invoke(["ui", "list"], tmp_path, monkeypatch) + + assert "2 of 120" in result.output + + @respx.mock + def test_it_is_free(self, tmp_path, monkeypatch): + """No --yes, and the suite's assume-yes is not what carried it.""" + monkeypatch.delenv("PIXELLAB_ASSUME_YES", raising=False) + respx.get(f"{PIXELLAB_BASE_URL}/ui-assets").respond( + json={"ui_assets": [a_panel()], "total": 1} + ) + + result = invoke(["ui", "list"], tmp_path, monkeypatch) + + assert result.exit_code == 0 + + +class TestShowingOnePanel: + """R1.6, R1.7. The panel was paid for when it was made; reading it back is free.""" + + @respx.mock + def test_the_image_is_written_to_the_workspace(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/ui-assets/ui-1").respond(json=a_panel()) + respx.get(PANEL_URL).respond(content=png_bytes(192, 192)) + + result = invoke(["ui", "show", "ui-1"], tmp_path, monkeypatch) + + assert result.exit_code == 0 + [written] = list((tmp_path / "out").glob("*/*.png")) + assert written.read_bytes() == png_bytes(192, 192) + + @respx.mock + def test_it_costs_nothing_and_asks_for_no_agreement(self, tmp_path, monkeypatch): + monkeypatch.delenv("PIXELLAB_ASSUME_YES", raising=False) + respx.get(f"{PIXELLAB_BASE_URL}/ui-assets/ui-1").respond(json=a_panel()) + respx.get(PANEL_URL).respond(content=png_bytes(192, 192)) + + result = invoke(["ui", "show", "ui-1"], tmp_path, monkeypatch) + + assert result.exit_code == 0 + assert "0 generations" in result.output + + @respx.mock + def test_a_panel_still_being_made_writes_nothing_and_says_so(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/ui-assets/ui-1").respond( + json=a_panel(status="processing", image_url=None, progress_percent=40, eta_seconds=12) + ) + + result = invoke(["ui", "show", "ui-1"], tmp_path, monkeypatch) + + assert result.exit_code == 0 + assert "no image yet" in result.output + assert "40%" in result.output + assert not list((tmp_path / "out").glob("*/*.png")) + + @respx.mock + def test_a_dry_run_sends_nothing(self, tmp_path, monkeypatch): + route = respx.get(f"{PIXELLAB_BASE_URL}/ui-assets/ui-1") + + result = invoke(["--dry-run", "ui", "show", "ui-1"], tmp_path, monkeypatch) + + assert result.exit_code == 0 + assert not route.called diff --git a/tests/test_pixellab.py b/tests/test_pixellab.py index f4ed5d2..b1817ab 100644 --- a/tests/test_pixellab.py +++ b/tests/test_pixellab.py @@ -20,7 +20,12 @@ RateLimited, ValidationError, ) -from pixellab_cli.pixellab import PixelLabClient, _decode_images, _usage +from pixellab_cli.pixellab import ( + MAX_DOWNLOAD_BYTES, + PixelLabClient, + _decode_images, + _usage, +) CREDENTIALS = Credentials(pixellab_secret="pl-test-token") @@ -406,3 +411,142 @@ def test_a_usage_without_seconds_leaves_them_unset(self): assert usage is not None assert usage.seconds is None + + +class TestAJobThatReturnsAnAddressRatherThanBytes: + """R4.9. `create-ui-asset` completes into `/ui-assets/{id}`, whose payload carries + `image_url` and no base64 at all — a real one, read off the account: + + {"id": "43d70268-…", "status": "completed", + "image_url": "https://backblaze.pixellab.ai/file/…/full.png?v=1789936808"} + + Thirty generations were charged for a panel that was never collected.""" + + PANEL_URL = "https://backblaze.pixellab.ai/file/pixellab-characters/ui/x/y/full.png?v=1" + + def a_completed_panel(self): + respx.post(url("/create-ui-asset")).respond( + json={"ui_asset_id": "ui-1", "background_job_id": "job-1", "status": "processing"} + ) + respx.get(url("/ui-assets/ui-1")).respond( + json={ + "id": "ui-1", + "status": "completed", + "size": {"width": 192, "height": 192}, + "image_url": self.PANEL_URL, + } + ) + + @respx.mock + def test_the_image_is_fetched_from_the_address(self, client): + self.a_completed_panel() + respx.get(self.PANEL_URL).respond(content=png_bytes()) + + result = client.call("create-ui-asset", description="a wooden panel") + + assert result.images == [png_bytes()] + + @respx.mock + def test_the_address_is_fetched_without_the_bearer_token(self, client): + """These links carry their own access in the identifier, like a rotation URL.""" + self.a_completed_panel() + asset = respx.get(self.PANEL_URL).respond(content=png_bytes()) + + client.call("create-ui-asset", description="a wooden panel") + + assert "authorization" not in asset.calls.last.request.headers + + @respx.mock + def test_bytes_in_the_payload_are_still_preferred(self, client): + """A payload carrying both is not fetched twice: the bytes are already here.""" + respx.post(url("/create-ui-asset")).respond( + json={"ui_asset_id": "ui-1", "background_job_id": "job-1", "status": "processing"} + ) + respx.get(url("/ui-assets/ui-1")).respond( + json={"status": "completed", "images": [image_payload()], "image_url": self.PANEL_URL} + ) + asset = respx.get(self.PANEL_URL).respond(content=png_bytes()) + + result = client.call("create-ui-asset", description="a wooden panel") + + assert result.images == [png_bytes()] + assert not asset.called + + +class TestCollectingAJobThatReturnsAnAddress: + """R4.9 through the resume path. `create-ui-asset` can time out, and what it tells + the caller to run is `pixellab-cli job show ` — which collects by id alone. A + charged panel must not be lost one hop later than the one this fix closed.""" + + PANEL_URL = "https://backblaze.pixellab.ai/file/pixellab-characters/ui/x/y/full.png?v=1" + + @respx.mock + def test_the_image_is_fetched_from_the_address(self, client): + job = "5bf5ee44-06e1-4a6f-9e5a-1e0e5ef0a6d1" + respx.get(url(f"/background-jobs/{job}")).respond( + json={"status": "completed", "last_response": {"image_url": self.PANEL_URL}} + ) + respx.get(self.PANEL_URL).respond(content=png_bytes()) + + result = client.collect(job) + + assert result.images == [png_bytes()] + + +class TestAnIdentifierThatWouldLeaveItsPath: + """A path parameter is interpolated into the URL, and the request carries the + bearer token — so a dot segment or a separator in one is a URL of somebody else's + choosing, reached with this caller's credential. `collect` already refuses it for a + job id; every path parameter is refused the same way, before anything is sent.""" + + @pytest.mark.parametrize( + "identifier", + ["../characters/char-9", "ui-1/../../balance", "a\\b", "..", "ui-1/extra"], + ) + @respx.mock + def test_it_is_refused_before_the_request(self, client, identifier): + route = respx.get(url(f"/ui-assets/{identifier}")) + + with pytest.raises(ValidationError): + client.call("ui-asset", ui_asset_id=identifier) + + assert not route.called + + @respx.mock + def test_an_ordinary_identifier_still_goes_through(self, client): + respx.get(url("/ui-assets/ui-1")).respond(json={"id": "ui-1", "status": "completed"}) + + result = client.call("ui-asset", ui_asset_id="ui-1") + + assert result.raw["id"] == "ui-1" + + +class TestWhatADownloadWillHold: + """The address is the provider's and is fetched without anybody asking, so how + much is read is not the provider's decision to make.""" + + ADDRESS = "https://backblaze.pixellab.ai/file/pixellab-characters/ui/x/y/full.png" + + @respx.mock + def test_a_body_past_the_ceiling_is_refused(self, client): + respx.get(self.ADDRESS).respond(content=b"x" * (MAX_DOWNLOAD_BYTES + 1)) + + with pytest.raises(ProviderError) as refused: + client.download(self.ADDRESS) + + assert "ceiling" in str(refused.value) + + @respx.mock + def test_a_declared_length_past_the_ceiling_is_refused_as_well(self, client): + respx.get(self.ADDRESS).respond( + content=b"x", headers={"Content-Length": str(MAX_DOWNLOAD_BYTES + 1)} + ) + + with pytest.raises(ProviderError): + client.download(self.ADDRESS) + + @respx.mock + def test_an_ordinary_asset_comes_through(self, client): + respx.get(self.ADDRESS).respond(content=png_bytes()) + + assert client.download(self.ADDRESS) == png_bytes() diff --git a/tests/test_run.py b/tests/test_run.py index b84ad72..f1964d8 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -936,3 +936,51 @@ def test_agreement_is_spelled_the_ways_people_spell_it(self, tmp_path, monkeypat monkeypatch.setenv(ASSUME_YES_VAR, value) assert self._call(tmp_path).files + + +class TestAgreementIsForACallThatCanCostSomething: + """R6.5. The gate is for a call that can cost money. A route whose price is known + to be zero — reading a spritesheet, writing out a panel the account already holds — + was asking for --yes to fetch a file.""" + + def runner_without_consent(self, tmp_path): + workspace = Workspace(tmp_path / "out") + return Runner(workspace=workspace, ledger=Ledger(workspace), approved=False) + + def test_a_route_known_to_cost_nothing_needs_no_agreement(self, tmp_path, monkeypatch): + monkeypatch.delenv(ASSUME_YES_VAR, raising=False) + runner = self.runner_without_consent(tmp_path) + + runner.approve("pixellab", "ui-asset", {}, Cost(generations=0.0, source="reported")) + + def test_an_unknown_price_still_asks(self, tmp_path, monkeypatch): + """Not knowing a price is not the same as knowing it is nothing.""" + monkeypatch.delenv(ASSUME_YES_VAR, raising=False) + runner = self.runner_without_consent(tmp_path) + + with pytest.raises(ApprovalRequired): + runner.approve("pixellab", "create-ui-asset", {}, None) + + def test_a_price_of_zero_that_is_only_an_estimate_still_asks(self, tmp_path, monkeypatch): + monkeypatch.delenv(ASSUME_YES_VAR, raising=False) + runner = self.runner_without_consent(tmp_path) + + with pytest.raises(ApprovalRequired): + runner.approve("pixellab", "a-route", {}, Cost(generations=0.0, source="estimated")) + + def test_a_reported_price_that_is_not_zero_still_asks(self, tmp_path, monkeypatch): + monkeypatch.delenv(ASSUME_YES_VAR, raising=False) + runner = self.runner_without_consent(tmp_path) + + with pytest.raises(ApprovalRequired): + runner.approve("pixellab", "a-route", {}, Cost(generations=30.0, source="reported")) + + def test_a_time_priced_route_reporting_no_generations_still_asks(self, tmp_path, monkeypatch): + """fal reports seconds and no generations; that is a price, not a zero.""" + monkeypatch.delenv(ASSUME_YES_VAR, raising=False) + runner = self.runner_without_consent(tmp_path) + + with pytest.raises(ApprovalRequired): + runner.approve( + "fal", "a-route", {}, Cost(generations=0.0, seconds=4.0, source="reported") + ) From 5f73cc70165aad0a50c7a61720d8c9d64dfd20cc Mon Sep 17 00:00:00 2001 From: prode Date: Sun, 20 Sep 2026 18:01:48 -0300 Subject: [PATCH 2/2] chore(specs): record the branch and the pull request --- specs/asset-workspace/requirements.md | 4 ++-- specs/interface-portrait-and-font/requirements.md | 6 +++--- specs/provider-core/requirements.md | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/specs/asset-workspace/requirements.md b/specs/asset-workspace/requirements.md index 0f37245..81d955c 100644 --- a/specs/asset-workspace/requirements.md +++ b/specs/asset-workspace/requirements.md @@ -1,9 +1,9 @@ --- autonomy: auto ci: wait -branch: feat/one-animation-many-directions +branch: feat/the-ui-panel-you-paid-for delivery: in-review -pr: 78 +pr: 79 --- # Asset workspace — requirements diff --git a/specs/interface-portrait-and-font/requirements.md b/specs/interface-portrait-and-font/requirements.md index fa79ec7..fd52bf9 100644 --- a/specs/interface-portrait-and-font/requirements.md +++ b/specs/interface-portrait-and-font/requirements.md @@ -1,9 +1,9 @@ --- autonomy: auto ci: wait -branch: feat/foundation -delivery: merged -pr: 52 +branch: feat/the-ui-panel-you-paid-for +delivery: in-review +pr: 79 --- # Interface, portrait and font — requirements diff --git a/specs/provider-core/requirements.md b/specs/provider-core/requirements.md index 0ad976d..650bad9 100644 --- a/specs/provider-core/requirements.md +++ b/specs/provider-core/requirements.md @@ -1,9 +1,9 @@ --- autonomy: auto ci: wait -branch: fix/paid-call-defects -delivery: merged -pr: 13 +branch: feat/the-ui-panel-you-paid-for +delivery: in-review +pr: 79 --- # Provider core — requirements