feat(crews): trait-by-trait custom ghost avatars for crew members

[CrysisDeu]

Problem / Motivation

Crew members are positioned as person-like identities — a name, a face, their own memory — but the face is fixed: it is derived deterministically from the crew's name, and renaming the crew is the only way to change it. Users cannot shape the identity they are meant to relate to.

Why it matters

The Members page makes named crew members the primary object users interact with. A face users chose themselves makes members recognizable at a glance across the roster, DM threads, and the editor — and keeps the identity stable even when two crews have similar names.

What changed (motivation → approach → change)

This PR ships the complete per-crew custom avatar feature in two tiers. The second tier was developed as the stacked PR #7702 (reviewed and approved there by @chenmingwei23 and @iamwhatever, merged into this branch), and this description covers both. The branch is a single squashed commit rebased onto current main.

Tier 1 — trait-by-trait ghost builder

Goal: let users customize a member's face without leaving the Kiro ghost family (keeps the roster visually coherent), stored per-crew with zero migration.

Approach: the ghost style module already separates trait drawing from trait picking — compose(traits) is the exported, test-exercised composition path, and the PRNG is only used to pick traits from the name. So a trait-by-trait builder needs no new art pipeline: it recombines the exported trait vocabulary (13 eyes, 4 brows, 9 mouths, 12 headwear, 8 items, blush, flip, 15 tiles) and pins the chosen set on the crew record as a sparse override, exactly like the existing per-crew model override. Empty = today's name-derived face.

• Backend: avatar field on KiroCrewAgentConfig with a total _safe_avatar coercer (now in config/sections.py, where main moved the config dataclasses; loader.py re-exports it). Junk collapses to "no override"; tile is pinned to hex because it is interpolated into SVG; booleans require real booleans since bool("false") is True; an all-empty ghost trait set collapses to the canonical {} reset spelling. Create/update endpoints validate with a 400 invalid_avatar code, mirroring session_color. The Members roster allowlist exposes the field.
• Frontend: CrewAvatar renders pinned traits through the same compose() path (composes with the working animation from feat: animate crew member avatars while a member is working #6979); CrewAvatarBuilder is a nested game-style dialog in the crew editor — category tabs per trait axis (each with a lucide icon so the collapsed form on narrow viewports stays legible), live preview, randomize, "Reset to the default face", and an "Applies here — press Save changes to keep it" hint. Entry points: the editor header avatar and the Overview hub avatar (both clickable) and an Avatar row in the Triggers pane. ghostDataUri lives in the style .ts module so no <svg literal appears in any .tsx. Tile swatch aria-labels are human color names, not hex.

Tier 2 — uploaded picture

Goal: an arbitrary picture per crew (photo, team logo, existing art), with the same two-step Apply → Save semantics as ghost traits, and no way for a failed or abandoned Save to damage the previously saved picture.

Approach: staged upload + atomic promote, with the ordinary crew PUT as the single commit point.

• HTTP surface (routes/agents.py, handlers/agents.py):
  • POST /api/agents/{name}/avatar — stages the file only (<sha256(name)>.pending.<ext>). Owner-gated via _require_owner("agent.avatar_upload") and audited via log_api_access. Body is multipart, read into memory under a 1 MB cap (413 avatar_too_large); format is decided by magic-byte sniffing (client Content-Type is ignored; PNG/JPEG/WEBP only, else 400 avatar_bad_format); malformed multipart → 400 invalid_multipart; unknown crew → 404 agent_not_found. Returns a staging token.
  • GET /api/agents/{name}/avatar — owner-gated (agent.avatar_get), audited, serves only the file the config's file pin selects and only while the config says kind: image; content-hash ETag with 304 revalidation. A leftover file the config no longer selects is refused.
  • No DELETE route. Removal is the same commit point: PUT /api/agents/{name} with avatar: {} (or any non-image override) clears the field and reaps the stored files after the config write succeeds.
• Storage and lifecycle: files live under the data home's agent-fenced run/avatars/ directory; filenames are sha256 digests of the crew name (display strings never touch a path). On PUT with {"kind":"image","promote":true,"token":…}, under the config lock: the staged file is installed at a content-addressed path (<stem>.<16-hex digest>.<ext>, so an install can never overwrite the committed file), the config is saved with {"kind":"image","v":<mtime_ns>,"file":"<digest>.<ext>"}, then the previous variants are reaped. If the config save raises, the freshly installed file is rolled back and the prior pin stays live. A PUT with a plain {"kind":"image"} keeps the current picture and discards any stale staging; a stale or missing token fails the save (400 avatar_file_missing) rather than silently keeping the old picture. Crew deletion reaps live and staged files inside the same lock. All filesystem work runs off the event loop.
• Wire shape: kind now has two constructed variants — {"kind":"ghost","traits":{…}} and {"kind":"image","v":int,"file":"<digest>.<ext>"} — validated by _safe_avatar (v must be a positive real int, file must match the digest-pin regex).
• Frontend: the builder gains Ghost face / Picture mode tabs. The Picture pane drag-drops or picks a file, center-crops square and downscales to 512 px client-side with a size ladder (PNG → white-ground JPEG → smaller JPEG) so any decodable pick fits the 1 MB server cap; a pick-generation guard stops a slow decode from overwriting a later pick. saveEdit snapshots the sheet epoch before its first await, stages the upload behind a fieldset fence that disables the editor pane during any in-flight save, and lets the PUT commit. CrewAvatar renders kind: image from the authenticated endpoint with the ?v= cache-buster and falls back to the seeded ghost if the load fails.

Shared

• i18n: 85 keys under components.avatarBuilder across all 13 catalogs (option labels use literal key maps, no dynamic keys) + en-XA regeneration + context sidecar entries.
• config-baseline.json regenerated on this head — the agents.*.avatar help now matches the loader's _meta string including the image-tier clause (scripts/generate_config_baseline.py yields zero drift).
• Rebase notes (433 commits of main): main moved the config dataclasses to config/sections.py, so _safe_avatar, the _AVATAR_* constants and the avatar field live there; per test_config_module_boundaries the loader's name-level re-export list is a frozen pre-split snapshot, so the handler and tests import them from config.sections directly and the loader reaches _safe_avatar through the module. The crew PUT/DELETE handlers merge main's reasoning_effort validation and _refresh_session_defaults calls with this PR's drained save + avatar commit/rollback; the Members drawer's identity moved into main's DetailPanel header, which now receives avatar={active.avatar}; two jsx-a11y warnings on the new drop zone were addressed (justified per-line disable on the drag-only div, aria-label on the hidden file input) to meet main's zero-warning eslint gate.
• CI round-2 fixes: main's whole-surface ApiClient.coverage test now has a Blob-fixture hand test for uploadCrewAvatar; the Crews row of docs/feature-map/README.md lists the avatar capability and endpoints; the upload pane's pick error renders through ErrorNotice (no hand-off, the dialog holds the unsaved avatar draft); and the upload endpoint now rejects a structurally incomplete body — valid magic bytes but no PNG IEND / JPEG FFD9 terminator, or a RIFF length that does not match — with 400 avatar_bad_format, so a truncated upload can never be promoted over the committed picture (_image_body_complete, dependency-free; a full decoder is not a dependency of this package).
• CI round-3 (advisory-lane) changes: the module spec now carries the avatar surface — docs/system-specs/modules/learn-cron-dashboard.md (Crew avatars: both routes, the stage → content-addressed install → save → reap protocol, rollback, stale-token and no-promote semantics, GET pin-only serving) and docs/system-specs/modules/config.md (Per-crew avatar override: both accepted shapes and the coercion rules), per AGENTS.md's same-commit spec rule; the Overview hub face now opens the builder like the header face (hub-avatar-button, same title); and a transport-level upload exception shows the localized failure string in the editor banner with the raw message kept for the console.
• CI round-4 changes: a record without a valid file pin (only reachable by hand-editing config.json) now selects no file — GET 404s into the ghost fallback and a picture-keeping save refuses with avatar_file_missing — instead of falling back to "any stored variant", which could have served an orphaned install left by a crash between install and save (GPT lane); the unreferenced _avatar_path resolver is deleted; and while the Picture pane is open, dragover/drop are cancelled at the window so a file dropped outside the dashed zone no longer navigates the SPA away from the editor (UX lane). Both covered by tests.
• CI round-5 change: the editor header avatar button — the one avatar entry point outside the pane's <fieldset> busy fence — now carries disabled={sheetBusy}, so the builder cannot be reopened while an upload or the committing PUT is in flight and a newer Apply can no longer be discarded by the completing save's close (GPT lane).

Tests

• test/test_agent_avatar.py (66 tests on this head, incl. pinless-record refusal, _image_body_complete table + truncated-upload rejection): _safe_avatar shape/coercion table (junk, unknown kinds, string-typed booleans, tile injection attempt, image v/file pins, all-empty ghost → reset), dataclass defaults + asdict round-trip, config load round-trip, ghost endpoint tests (persist, 400 on junk, reset via {}, create-with-override), and the upload-tier endpoint tests: stage → commit → serve roundtrip with 304 revalidation; abandoned staging never changes what is served; commit without upload → 400; stale token fails the save; the live picture stays on disk through every promotion step (one spy in this test was tightened on this head to observe only the install step, not the staging write that also lands at a .jpg path); magic-byte sniff beats a lying Content-Type; 1 MB cap → 413; unknown crew → 404; malformed multipart → 400; format change leaves no stale sibling; same-size replacement changes the ETag; crew deletion reaps live and staged files; GET refuses a file the config no longer selects.
• test/test_config_baseline.py: committed snapshot is byte-identical to the generator on this head.
• test/test_members_dm_thread.py: pins avatar in the roster allowlist.
• website/src/test/CrewAvatarBuilder.test.tsx (17 tests, the builder on its own): randomize draws from the shipped vocabulary; Blush and Background axes; mirror toggle; Cancel; the picture tier's crop/downscale ladder (PNG → JPEG on a white ground → smaller JPEG, no upscaling), the 20 MB source cap and undecodable/zero-size/no-canvas failures through ErrorNotice, the drop zone, the hidden-input forwarding, the pick-generation guard (a slow earlier decode cannot overwrite a later pick), reopening over a saved picture, and Reset. Lifts the file to 99% line coverage for the per-file floor.
• website/src/test/CrewRoster.test.tsx (52 tests): pinned override changes the face deterministically; junk falls back to seeded; non-hex tile never reaches the SVG; seededTraits parity; builder E2E (open from header avatar → pick eye → Apply → Save payload carries the traits; reset round-trips to "no override"); image override renders the authenticated URL with the cache stamp; draft picture previews from its data URI without network; failed image load falls back to the seeded ghost; Picture tier disables Apply until a picture exists and the ghost draft survives tab round-trips.

What ran locally against this head (c2ea7d16d)

Black gate, isort, flake8, mypy (config + handler modules), the four baselined backend gates (subprocess-encoding, agent-sdk boundary, sync-io-in-async, lockdown-before-publish), brand gate, scrub-lint, tsc -b, eslint --max-warnings 0, i18n:check, phantom-classes, jscpd — all pass. Targeted tests: test_agent_avatar.py + test_config_baseline.py + test_members_dm_thread.py plus test_config_module_boundaries.py (148 passed), CrewRoster.test.tsx (52 passed), ApiClient.coverage.test.tsx (696 passed), and the existing KiroCrewAgentsPage.* suites (25 passed). The full backend and frontend suites are left to CI, which is authoritative.

Manual verification

Both tiers were walked on an isolated pod (built dist, real gateway) at the time each was built: ghost tier — created crews via the API, pinned a custom face through the update path, roster/editor/builder all reflect it live; picture tier — picture picked → center-cropped preview → Apply → Save changes → roster shows the uploaded face. The screenshots below are from those sessions; the code paths they exercise are unchanged by the rebase (the conflict resolution touched module placement and the merge of main's reasoning_effort handling, both covered by the tests above).

Screenshots / video

More surfaces

Issue link

no linked issue: feature built from a dashboard design session with the maintainer; the picture tier was tracked as stacked PR #7702.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of c2ea7d16db8895538263c23c00d806c0da5c4927 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Both tiers reuse existing seams (per-crew override precedent, compose path, authenticated serving), and the staged-upload→PUT-commit protocol earns its complexity against the commit-on-upload alternative.

Suggestions

• The image tier's store lifecycle (~350 lines: stem/variant/pending resolution, promote/commit/rollback/reap, _drained_to_thread) is a self-contained content-addressed store living inside the already-large handlers/agents.py; extracting it to its own module (e.g. dashboard/avatar_store.py) would keep the HTTP handlers readable and make the transaction protocol independently testable.

[DESIGN-REVIEWED] c2ea7d1

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of c2ea7d16db8895538263c23c00d806c0da5c4927 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All mechanical checks are done. Composing the review.

First-Principles-Verdict: CONCERNS

Every capability earns its place, but two slivers of wire/API surface ship with zero or one consumer: create-time avatar and the redundant promote key.

What this change ships

Intent: let a user choose a crew member's face — hand-picked ghost traits or an uploaded picture — instead of the name-derived one. ADDITION.

1. Crew editor gains an avatar-builder dialog (trait tabs, randomize, reset) — justified
2. Editor header and Overview hub faces become clickable openers of that dialog — justified (cited first-run report)
3. New Avatar row in the Triggers pane as the text route — justified
4. Picture tier: pick/drag a photo, client-side crop to 512 px, staged upload committed by Save — justified
5. New config field agents.*.avatar with load-time coercion, spec + baseline updated same commit — justified
6. Two new owner-gated, audited endpoints GET,POST /api/agents/{name}/avatar — justified
7. POST /api/agents (create) also accepts avatar — zero consumers
8. Wire-only promote + token keys on the committing PUT — one consumer, generalized
9. Whole editor pane now disabled during any in-flight save — justified (the upload lengthens the save window the fence protects)
10. Members roster responses carry avatar; 85 i18n keys × 13 catalogs — justified

Watch

• promote is derivable from token's presence: the sole producer (saveEdit, KiroCrewAgentsPage.tsx) only ever constructs {promote: true, token} together, promote-without-token is a 400, and token-without-promote behaves like no-promote. One wire key is carrying no information the other doesn't; tokens never persist, so token-presence is already unambiguous.
• _is_ghost_shaped (handlers/agents.py) re-states _safe_avatar's ghost structure (kind/traits/type checks) to tell junk from reset at the 400 gate — the trait constants are shared but the structural logic now has two spellings that must not drift.

Subtractions

• Drop avatar from api_kirocrew_agents_create: the dashboard's create call sends no avatar (the builder is edit-only; createMut.mutate in KiroCrewAgentsPage.tsx carries session_color but not avatar — grepped, 0 callers), and the image branch there exists only to refuse itself. Create-then-edit already covers the job; the update path keeps all the validation.
• Drop the promote wire key; treat a present token as the promote directive (one producer: KiroCrewAgentsPage.tsx saveEdit; delete the key from the handler, CrewAvatarOverride, the spec, and the tests).
• Replace the _safe_avatar + _is_ghost_shaped pair with one classifier in config/sections.py returning valid/reset/junk; the loader maps junk→{}, the endpoints map junk→400 — one schema spelling instead of two.

[FIRST-PRINCIPLES-REVIEWED] c2ea7d1

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

UX-level review of c2ea7d16db8895538263c23c00d806c0da5c4927 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: CONCERNS

Solid builder with honest Apply→Save layering, but the Avatar setting hides inside the Triggers pane and the category tabs degrade to cryptic icon-only at common window sizes.

Watch

• Avatar filed under "How work arrives → Triggers" (KiroCrewAgentsPage.tsx, the <Field label={…field_label}> added inside the Triggers pane; screenshot 03-routing-pane.png). The rail has no "Avatar" entry, so a user scanning "Who it is" finds nothing, and an avatar edit marks its unsaved dot on Triggers (out.add('routing')) — the dot names a section the user never touched. Frequency moderate, friction not failure, recurs every edit; the clickable header/hub faces soften it. Smallest fix: move the field (and its dirty flag) to the Overview pane, or give it its own rail row.
• Seven axis tabs collapse to icon-only at ordinary widths (06-face-tab-settled.png, 1280×860: Meh=Brows, Coffee=Item, Heart=Blush, Crown=Headwear). None of these icons are self-evident, so a first-time user identifies tabs only by clicking through or hovering — and the dialog's fixed 760px width means the labeled form (04-builder-eyes.png) appears only marginally wider. Every builder visit, comprehension friction. Smallest fix: let the strip wrap to two labeled rows inside the builder instead of collapsing (collapse={false} is already used for the mode switch).

Suggestions

• Applying a Picture over a hand-built ghost face silently drops the trait draft: reopening the builder after Apply seeds draft from value?.kind === 'ghost' only, so the crafted face is gone unless the whole editor is cancelled. Preserve the last ghost traits as the Ghost face tab's seed within the editor session.

[UX-REVIEWED] c2ea7d1

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed c2ea7d16db8895538263c23c00d806c0da5c4927 — this comment is updated in place on each push.

Review details

The candidate is disproven. The security layer fences run/ read and write from agent file tools — security.py:6652 lists "run" in the data-home sensitive-dir set, with an explicit comment that a "prompt-injected / sandboxed agent that could WRITE into this dir" is the exact threat the fence closes. So the candidate's trigger (a) — avatar bytes replaced out-of-band by an agent file-write — cannot occur via agent tools; the run/avatars/ write-fence the docstring asserts is real and enforced. The candidate's own confidence line already conceded this was conditional ("if the run/ write-fence is not enforced elsewhere"), and it is enforced. Harm was also bounded (Content-Type locked to image/*, no script execution), so even absent the fence it is not a blocking class. Candidate dropped.

No grounded Step 2 finding at 80+ in the code I opened.

No findings.

[OPUS-REVIEWED] c2ea7d1

Verdict parsed from the review's SHA-scoped output markers for commit c2ea7d16db8895538263c23c00d806c0da5c4927.

False positive or not applicable? A repository writer can comment:
/ai-review override fable c2ea7d16db8895538263c23c00d806c0da5c4927: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @CrysisDeu overrides the GPT 5.6 finding for c2ea7d16db8895538263c23c00d806c0da5c4927; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

False positive or not applicable? A repository writer can comment:
/ai-review override gpt c2ea7d16db8895538263c23c00d806c0da5c4927: <one-sentence reason>

[CrysisDeu]

• avatar controls are clipped at 320px — span=7cec18d4da21 — fixed in 2b71443.

<SegmentedControl ... collapse={false} /> and <DialogFooter> — 320px viewport -> overflow-hidden dialog -> later trait tabs and translated Apply action become unreachable.

Two changes: the builder's SegmentedControl now uses its default measured collapse (full -> compact -> dropdown), so all 7 trait tabs stay reachable at phone width; and the builder's DialogFooter gets flex-wrap with min-w-0 on the reset/hint block, so the Cancel/Apply actions wrap onto their own row instead of being pushed past the dialog edge.

[CrysisDeu]

Status note — the 5 red checks on this head are inherited from main, verified on a pristine origin/main (dd9e002) checkout:

• Backend Tests (3.10/3.12/Windows, shard 3): test_security_posture.py::TestGateSideLogRedactorSpelling::test_the_census_holds_no_slack fails identically on unmodified origin/main (_BASELINE_LOG_SITE_CENSUS says dashboard/handlers/files.py: 3 sites, the code now has 1 — census drift from a merged PR; this branch touches neither).
• Frontend Lint & Type Check: pristine origin/main yields 660 eslint warnings against the ratchet of 659 (a merge race between two PRs); this branch's files contribute 0 new warnings (verified per-file against base).
• Coverage Gate: downstream of the failed backend shards.

The one genuine finding on this PR (GPT lane 320px clipping) was fixed in 2b71443. I'll rebase and re-push once main is green again.

[CrysisDeu]

Finding: temp-screenshots (~1.6MB) committed into history — rejected as a change to this PR: temp-screenshots/ is this repo's documented convention for PR media (temp-screenshots/README.md; the QA workflow mandates committed, commit-SHA-pinned raw URLs precisely so PR-description images cannot rot). The binaries-in-history trade-off is the convention's accepted cost, applied uniformly across PRs.

[CrysisDeu]

Finding: _safe_avatar accepts {"kind":"ghost","traits":{}} as a third state — valid observation, deferred: the builder UI cannot produce an all-empty trait set (Apply always carries the seeded defaults), so the state is reachable only via direct API use. Collapsing it belongs with the validator changes the upload tier already requires; folding into that follow-up PR.

[CrysisDeu]

Finding: spec gap on the new public surface — fixed in 2c098686e. docs/system-specs/modules/learn-cron-dashboard.md gains a Crew avatars paragraph next to Crew model pins covering both routes, the stage → content-addressed install → config save → reap ordering, rollback on a failed save, why a PUT without promote discards staging, why a stale or missing token fails the save, GET's pin-only serving, and the closed image-tier create path; docs/system-specs/modules/config.md gains Per-crew avatar override with both accepted shapes and every coercion rule (including why _safe_avatar is reached through the sections module rather than the frozen loader re-export list).

Suggestion: promote _drained_to_thread to every cfg.save() caller — accepted as a follow-up, not folded in here: the other callers' saves are not inside a filesystem commit/rollback protocol, so the cancellation hazard the helper closes is only load-bearing on the avatar paths today; widening it touches every mutation in the module and belongs in its own reviewed change.

[CrysisDeu]

Finding: discoverability — the Overview hub face is inert — fixed in 2c098686e with the smallest fix you named: the hub node is now a button that opens the builder, carrying the same title/aria-label as the header face (hub-avatar-button). The Triggers-pane Avatar row stays as the labeled text route.

Finding: raw exception text in the editor banner — fixed in 2c098686e for the path you cited: a thrown upload error (network drop, decode failure) now shows the localized failed_to_update_agent string and the raw message goes to the console. Server-provided error strings on a non-2xx (up.error, and the PUT race message via updateMut.onError) are left as-is on purpose — that is the page-wide convention for every other crew field, and changing it only for avatar would make the avatar banner the one surface that hides the server's reason.

Suggestion: customized_note "Using a custom face" beside a picture — accepted, deferred: it is one string across 13 catalogs, and the i18n gates refuse an English-only edit; batching it into the next catalog sweep rather than paying a full CI round for a caption.

[CrysisDeu]

Finding / Subtraction: POST /api/agents accepts a ghost avatar that no shipped UI sends — rejected, with the reasoning made explicit rather than "symmetry": the create endpoint's consumers are not only the SPA. Crews are created through the API by agents and scripts (the manual-verification section of this PR created its crews that way, and the roster/DM surfaces are built for agent-driven crews), and for that caller a create-with-face is a single call where the alternative is create → read back → update. The refusal you propose would not remove a branch so much as move it: a caller that sends avatar on create would need the same validation to produce the same 400, and the one behaviour worth closing — silently dropping a supplied override — is already closed by the existing 400 on junk. The image branch stays a flat 400 (avatar_file_missing) because there it is structurally impossible to satisfy; the ghost branch is satisfiable and covered by test_create_accepts_an_override. Kept.

[CrysisDeu]

Finding: pinless image config can serve an uncommitted avatar — fixed in 83675b315. _live_avatar_file no longer falls back to "any stored variant" when the record carries no valid file pin: a pinless {"kind":"image"} (only reachable by hand-editing config.json — every writer stamps the pin at the commit) now selects nothing, so GET answers 404 (the roster falls back to the seeded ghost) and a picture-keeping save on it refuses with 400 avatar_file_missing instead of adopting an unknown file. The now-unreferenced _avatar_path fallback resolver is deleted. Covered by test_pinless_image_record_serves_nothing, which plants an orphan digest-named variant and asserts it is neither served nor adopted nor silently deleted.

[CrysisDeu]

Finding: a stray drop wipes the editor — fixed in 83675b315: while the Picture pane is showing, dragover/drop are cancelled at the window (CrewAvatarBuilder.tsx), so a file that misses the dashed zone — the preview image, the dialog body, the page — no longer navigates the SPA to the file. The listeners attach only for the pane's lifetime and detach on tab switch and unmount; the drop zone's own handlers are unchanged. Pinned by a test that dispatches drops on document.body in each state.

Finding: an applied avatar discards silently on editor close — accepted as valid, deferred with the concrete reason: the discard confirm main added guards schedDraft with schedule-specific copy (discard_new_schedule*); extending it to the avatar draft — or to dirtyPanes.size > 0, which is the coherent version — needs editor-wide copy across 13 catalogs and a product decision about every pane's dismissal (a long prompt edit is lost identically today). Filing it as the follow-up that generalizes the existing schedule guard, rather than smuggling an editor-wide behaviour change into an avatar PR.

Suggestion: Brows (Meh) and Mouth (Smile) icons are near-identical at 13px — accepted, deferred to the same follow-up; a glyph swap is a one-line change but it re-runs the screenshot evidence for the compact strip.

[CrysisDeu]

Subtraction: drop _avatar_path and the no-pin fallback in _live_avatar_file — accepted and done in 83675b315 (the same defect the GPT lane blocked on): a pinless {"kind":"image"} now 404s into the existing ghost fallback and _avatar_path is deleted.

Watch / Subtraction: _drained_to_thread duplicates run_config_write's drain loop — valid observation, rejected as a change to this PR: run_config_write acquires _get_config_lock() itself, while the avatar commit runs several drained steps (install → save → reap, or rollback) inside one already-held lock, so it cannot call run_config_write and the shared piece would have to be a new lock-free primitive extracted from chat_utils that run_config_write then delegates to. That is a refactor of main's helper with its own callers and tests, not avatar work; the two loops are equivalent today (both absorb every cancellation and re-raise once — one re-raises the original exception object, one a fresh one, which no caller distinguishes). Filed as the follow-up that owns the extraction.

Watch: the drained save lands on update/delete only — noted, out of scope: agents.py:2751 (sync) and the create handler's cfg.save() are main's pre-existing on-loop saves with no filesystem transaction around them; the avatar paths use the drained form because a cancellation there would release the lock mid-transaction. Converting the siblings belongs with the extraction above.

Watch / Subtraction: ghost avatar on POST /api/agents has zero shipped callers — rejected, same grounds as the previous head (crews are created through the API by agents and scripts, and a supplied override must be validated identically whether it is accepted or refused; the image branch already 400s). Kept.

[CrysisDeu]

Design lane is PASS on this head; two suggestions, both accepted as follow-ups:

Extract the avatar file-store protocol from handlers/agents.py — agreed; it pairs with the First-Principles lane's ask to share the drain loop with run_config_write, and both belong in one extraction PR rather than a second large move inside this one.

Long max-age on the ?v=-versioned GET — agreed in principle; the URL is content-versioned by mtime and the pin, so a long cache is safe. Deferred because a long max-age also caches the owner-gated response in the browser across a sign-out, which wants a private directive and a look at how the other authenticated image routes handle it before changing the header here.

[CrysisDeu]

Finding: header avatar remains editable during saves — fixed in c2ea7d16d: the header avatar button now carries disabled={sheetBusy} (it sits outside the editor pane's <fieldset> fence, which already gates the Overview-hub face and the Triggers-pane Customize button), so the builder cannot be reopened while an upload or the committing PUT is in flight, and a newer Apply can no longer be discarded by the completing save's close.

[CrysisDeu]

Per-finding dispositions for the First Principles CONCERNS on 83675b315 (unchanged on c2ea7d16d, which only gates the header avatar button during saves):

Watch / Subtraction: _drained_to_thread is a third copy of the shield-and-drain loop (mcp.py:_offload_config_write, chat_utils.run_config_write) — valid, rejected as a change to this PR and filed as the extraction follow-up. Neither existing spelling is importable as-is for this call site: run_config_write acquires the config lock itself, and _offload_config_write is a private helper of a sibling handler module (importing it here makes agents.py depend on mcp.py for a concurrency primitive, which is the wrong home). The right fix is one shared lock-free primitive that all three delegate to — a refactor of main's helpers with their own callers and tests, not avatar work.

Watch / Subtraction: collapse promote+token into the file pin and delete the .pending.* lifecycle — rejected, with the reason the two-step exists: the pending slot is what keeps Apply side-effect-free. Installing straight to the digest path on upload makes every picked-but-never-saved picture a committed-looking install on disk (Cancel would have to reap, and a Cancel that never runs — tab closed — leaves it), and the current reap only runs at a successful save. The token is not defending a race the slot creates; it binds a specific PUT to the specific bytes this editor staged, so an overlapping save from another window cannot commit bytes it never previewed. That the token equals the eventual pin digest is a convenience, not a redundancy. Simplifying this is a design change with its own trade-offs; happy to take it in a follow-up with the owner's decision.

Watch: 7 sibling inline cfg.save() sites keep the on-loop behaviour — accepted and deferred (as the lane itself marks it); belongs with the extraction above.

Subtraction: drop the all-empty-ghost → reset acceptance and _is_ghost_shaped — valid, accepted and deferred. You are right that a flat 400 on every collapsing payload removes both the helper and the mistyped-traits-delete-the-picture hazard it exists to guard, and that no shipped sender produces the all-empty shape (the builder always carries seeded defaults; the SPA resets with {}). It is deferred rather than folded in because it reverses an acceptance rule the Design lane asked for in round 1 (collapse the third state to reset) and changes two endpoint contracts plus their tests, and this is the last fix round on this PR; it goes in the same follow-up as the drain-loop extraction, where the API-shape decision can be made once.

[CrysisDeu]

UX lane is PASS on this head. Suggestion: Randomize overwrites a hand-picked draft with no one-step revert — accepted as a follow-up (stash the pre-randomize draft and offer "Restore previous" on the reset row); it needs one new string across 13 catalogs, so it rides with the deferred customized_note copy fix and the Brows/Mouth icon swap rather than a CI round of its own.

[CrysisDeu]

Design lane is PASS on this head; both suggestions stand as previously dispositioned: long max-age on the ?v=-stamped GET (agreed; deferred until the private caching posture of the other owner-gated image routes is checked) and extracting the avatar file store from handlers/agents.py (agreed; it is the same follow-up as the First-Principles drain-loop extraction).

[CrysisDeu]

/ai-review override gpt c2ea7d1: Maintainer ruling: 5 rounds fixed every adjacent GPT finding (feature map, ErrorNotice, truncated-image check, pinless pin, busy fence). This one is not avatar-specific — description/triggers on the same record take the same unredacted path — so it needs one serializer-level redaction chokepoint, tracked as #8447 rather than a sixth round here.

[github-actions]

Human judgment recorded

@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for c2ea7d16db8895538263c23c00d806c0da5c4927.

Maintainer ruling: 5 rounds fixed every adjacent GPT finding (feature map, ErrorNotice, truncated-image check, pinless pin, busy fence). This one is not avatar-specific — description/triggers on the same record take the same unredacted path — so it needs one serializer-level redaction chokepoint, tracked as #8447 rather than a sixth round here.

This decision applies only to this commit. A new push requires a new judgment.

[CrysisDeu]

Superseding my earlier note on this head (the lane re-ran and moved from PASS to CONCERNS). Per-finding:

Finding: the Avatar field lives in the Triggers pane and dirties the "routing" dot — valid, accepted and deferred to the follow-up as an editor-structure change: giving the face its own rail row (or moving it to Overview with its own dirty flag) touches the pane registry, dirtyPanes, and the rail's labels across 13 catalogs. The two clickable faces (header and Overview hub, both opening the builder) are the shipped mitigation for discoverability; the mis-attributed unsaved dot is the concrete defect the follow-up should fix first.

Finding: seven axis tabs collapse to icon-only at ordinary widths — valid, accepted and deferred with the fix you named (let the strip wrap to two labeled rows inside the builder rather than collapsing). It needs a layout option on the shared SegmentedControl (today it only knows full → compact → dropdown), which is a component change other callers inherit, so it goes with the follow-up rather than the last fix round here. The lucide icons were added precisely so the compact form is never blank; agreed they are not self-evident.

Suggestion: applying a Picture drops the hand-built ghost draft for the rest of the editor session — accepted, deferred to the same follow-up (keep the last ghost traits as the Ghost-face tab's seed while the editor stays open; pairs with the pre-randomize stash from the previous round).

None of these is a BLOCK; the PR's remaining UX debt is tracked in one follow-up alongside the copy fix (customized_note), the Brows/Mouth icon swap, the randomize revert, and the close-guard generalization.

[CrysisDeu]

F2 errors-use-error-notice — fixed in 5df62ac12: the upload pane's pick error renders through ErrorNotice (variant="inline", dismissable). askAgent stays off with the required No hand-off: comment naming the concrete draft it protects: the picked-but-unapplied picture and, behind it, the editor's unsaved ghost traits.

[CrysisDeu]

F3 header-only sniffing — fixed in 5df62ac12: new _image_body_complete(ext, body) runs after the magic-byte sniff and refuses a body whose container is not closed (PNG without a trailing IEND chunk, JPEG without the FFD9 marker, RIFF length not matching the body) with 400 avatar_bad_format, so a truncated upload is never staged and can never be promoted over the committed picture. Dependency-free by design — a full image decoder is not a dependency of this package; what every truncation breaks is the terminator, and that is what is checked. Covered by a parametrized unit table plus an endpoint test asserting nothing is staged.

[CrysisDeu]

Status: CI is fully green on c2ea7d16d (60/60 checks, PR Readiness passed, MERGEABLE). The only remaining gate is the human review — @bolichen97, your CHANGES_REQUESTED review from f3d1a1816 is still the recorded decision; every one of its three points was addressed in the first round (disclosure of the picture tier in the description, config-baseline.json regenerated, verification evidence re-stated for the current head), so a re-review would be appreciated.

Summary of what changed since that review, across five CI rounds (single squashed commit throughout):

• Rebased onto current main (~440 commits); config dataclasses followed main into config/sections.py, with the loader's frozen re-export list left untouched.
• Blocking-lane fixes: feature-map Crews row; pick error via ErrorNotice with a no-hand-off note; structural image-completeness check on upload (_image_body_complete); pinless image records serve nothing; header avatar gated during saves.
• Test/coverage: CrewAvatarBuilder.test.tsx (18 tests) brings the builder to 99% line coverage; ApiClient.coverage hand test for the multipart upload; pinless-record and truncated-upload endpoint tests.
• Specs: avatar surface documented in learn-cron-dashboard.md and config.md.
• UX: Overview hub face opens the builder; window-level drop guard while the Picture pane is open; localized upload failure copy.
• Advisory-lane items not taken here are dispositioned per finding above and tracked as follow-ups; the one GPT finding overridden by maintainer ruling is filed as Crew record strings (avatar traits, description, triggers) reach dashboard JSON without credential redaction #8447 (serializer-level redaction of crew-record strings, not avatar-specific).

Not merging — leaving that to the reviewers.