Frontmatter
| title | feat(fleet): surface avatarUrl in the fleet cockpit DTO (#14777) |
| author | neo-opus-vega |
| state | Merged |
| createdAt | 3:38 PM |
| updatedAt | 3:51 PM |
| closedAt | 3:51 PM |
| mergedAt | 3:51 PM |
| branches | dev ← agent/14777-avatar-dto |
| url | https://github.com/neomjs/neo/pull/14778 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: This is a narrow additive DTO field that completes the already-established storage → DTO → AgentCard avatar path. The diff is one production line plus a focused null-contract unit test; local evidence and hosted CI are green.
Peer-Review Opening: Cycle-1 peer review for the AgentCard avatar-data bridge. I checked the source issue, storage path, DTO sanitizer, AgentCard consumer premise, exact-head diff, local unit evidence, hosted CI, and wire-format compatibility before scoring.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #14777, changed-file list, current
devfleetCockpitStatus,FleetManager.setAvatar,FleetRegistryService/KB context for metadata storage, PR #14774 AgentCard premise, and Memory Core prior-art sweep. - Expected Solution Shape: A correct bridge should hoist the existing
metadata.avatarUrlpresentation reference into a flat nullable cockpit row field, matchingdisplayName-style display fields. It must NOT create avatar assets, change other row fields, expose secrets from metadata, or require consumers to dig intorow.agent.metadata. Test isolation should prove present-avatar and absent-avatar contracts in the focused DTO unit spec. - Patch Verdict: Matches. The diff adds
avatarUrl: publicAgent.metadata?.avatarUrl ?? nullaftersanitizePayload(agent), leaves other row fields untouched, and adds a unit test for populated and unset avatar values. - Premise Coherence: coheres: verify-before-assert is satisfied by storage/render/bridge checks; the change preserves the two-hemisphere boundary by keeping a Body-side DTO row field dependency-light; no flat-peer/team or no-hold surface is affected.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #14777
- Related Graph Nodes: #14598, #14560, #14755, #14774,
src/ai/fleet/fleetCockpitStatus.mjs,FleetManager.setAvatar,AgentCard.avatarUrl
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
- Documented search: I actively looked for other row-field drift, secret-shaped metadata leakage, and a missing null contract for agents without avatars, and found no concerns.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing matches the diff; it is the DTO bridge only, not avatar asset creation or card-wall mounting.
- Anchor & Echo summaries: source comments/test prose correctly frame
avatarUrlas display data frommetadata.avatarUrl. -
[RETROSPECTIVE]tag: N/A. - Linked anchors: #14777 / #14755 / #14774 support the stated storage-to-render pipeline.
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A — KB surfaced the FleetControlBridge/FleetRegistry metadata path; local code supplied the exact DTO contract.[TOOLING_GAP]: N/A.[RETROSPECTIVE]: For card-grain display fields, the clean contract is a flat row field over sanitized public agent metadata. Consumers should bindrow.avatarUrl, not spelunk throughrow.agent.metadata.
🎯 Close-Target Audit
For every issue named as close-target, verify it does NOT carry the epic label:
- Close-targets identified: #14777 via PR-body
Resolves #14777; commit subject carries conventional(#14777)and the commit body has no hidden close keyword. - For each
#N: #14777 is notepic-labeled.
Findings: Pass.
📑 Contract Completeness Audit
- Originating ticket contains the scoped DTO contract: top-level
avatarUrlfrommetadata.avatarUrl,nullwhen unset, purely additive. - Implemented PR diff matches the scoped contract exactly, with unit coverage for both populated and absent avatar values.
Findings: Pass.
🪜 Evidence Audit
Findings: N/A — close-target ACs are fully covered by focused unit/static evidence; the live card-wall binding remains post-merge/sibling validation and is not claimed as delivered here.
📡 MCP-Tool-Description Budget Audit
Findings: N/A — the PR does not touch ai/mcp/server/*/openapi.yaml or MCP tool descriptions.
Wire-Format Compatibility Audit
- Change type: additive nullable DTO row field (
avatarUrl). - Backward compatibility: existing consumers continue to receive all prior row fields; agents without avatars receive
null, notundefined. - Secret/privacy boundary: value is derived from
sanitizePayload(agent); existing recursive secret-key stripping remains in force before the flat field is read. - Downstream expectation: AgentCard already binds an
avatarUrldisplay field; this supplies the row-side data without changing card identity semantics.
Findings: Pass.
🔗 Cross-Skill Integration Audit
- Does any existing skill document a predecessor step that should now fire this new pattern? No — this is an internal DTO field, not a workflow convention.
- Does
AGENTS_STARTUP.md§9 Workflow skills list need updating? No. - Does any reference file mention a predecessor pattern that should now also mention the new one? No.
- If a new MCP tool is added, is it documented in the relevant skill's reference payload? N/A.
- If a new convention is introduced, is the convention documented somewhere? N/A — no new convention.
Findings: All checks pass — no integration gaps.
🧪 Test-Execution & Location Audit
- Branch checked out locally: exact-head detached worktree at
f677cac35bfbb6f4af5bae1f15f0cef003a0cb07. - Canonical Location: changed unit test remains under
test/playwright/unit/ai/services/fleet/. - If a test file changed: ran the specific test file.
- If code changed: verified syntax, structure map, hosted CI, and focused unit evidence.
Findings: Tests pass. Local evidence: git diff --check origin/dev...origin/pr/14778, node --check src/ai/fleet/fleetCockpitStatus.mjs, npm run --silent ai:structure-map -- --root src/ai/fleet --files --loc, and NEO_CHROMA_PORT_TEST=18198 npm run test-unit -- test/playwright/unit/ai/services/fleet/fleetCockpitStatus.spec.mjs -> 6 passed. Hosted CI is green; GitHub reports MERGEABLE/CLEAN.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.
[ARCH_ALIGNMENT]: 96 - Correct dependency-light DTO placement, flat display-field contract, no core/Brain service import leakage, and nosrc/core/Base.mjs/src/Neo.mjstouch.[CONTENT_COMPLETENESS]: 94 - PR body and test describe the storage/render/bridge pipeline and the null contract; minor residual is that live wall verification remains a post-merge sibling validation.[EXECUTION_QUALITY]: 98 - One-line additive implementation after sanitization, focused test for present/absent cases, local checks green, hosted CI green.[PRODUCTIVITY]: 100 - Fully delivers #14777’s scoped bridge.[IMPACT]: 58 - Small but useful DTO completion that unlocks real avatar binding for AgentCard; not a broad architecture change.[COMPLEXITY]: 18 - Two files, one production field, one focused test.[EFFORT_PROFILE]: Quick Win - Low-complexity, high-leverage bridge in an already-shaped pipeline.
This is merge-eligible from my side.
Resolves #14777 · Refs #14598 (FM cockpit AgentCard) · Refs #14560 (FM cockpit UI/UX epic) · Refs #14755 (card component, PR #14774)
The avatar-data bridge — the last link that makes per-agent profile avatars flow end-to-end (@tobiu's directive: "we spend quite the effort to create one for each you").
Evidence: VBA of the full pipeline this turn —
FleetManager.setAvatar({id, avatarUrl})persistsmetadata.avatarUrlon the agent's registry def (ai/services/fleet/FleetManager.mjs).ImagetoavatarUrl.avatarUrlinto the row.What it changes
One line in
src/ai/fleet/fleetCockpitStatus.mjs: hoistavatarUrl: publicAgent.metadata?.avatarUrl ?? nullinto the row, beside the other flat display fields (displayName,githubUsername,harnessType). The card-wall bindsrow.avatarUrlexactly as it bindsrow.displayName— no digging intorow.agent.metadata.nullwhen unset (neverundefined— a clean bindable contract). Source verified:sanitizePayloadpreservesmetadatarecursively.Test Evidence
npm run test-unit -- test/playwright/unit/ai/services/fleet/fleetCockpitStatus.spec.mjs→ 6 passed (was 5). New test: a row hoistsavatarUrlfrommetadata.avatarUrl, and an agent with no avatar recorded surfacesnull.Post-Merge Validation
row.avatarUrl; the AgentCard card-wall (sibling leaf of #14598) binds each resident's avatar to real data, populated asFleetManager.setAvataris called per agent.Deltas from ticket
Exactly the scoped hoist + its test — purely additive (no other row field changes). The avatar assets themselves (the per-agent images the operator/team creates) are supplied as data via
setAvatar; this PR makes the pipeline carry them, it does not create them.Authored by Vega (@neo-opus-vega · Claude Opus 4.8 · Claude Code) — origin session 3bc21462.