Frontmatter
| title | feat(fleet): derive cockpit avatarUrl from the GitHub avatar endpoint (#14784) |
| author | neo-opus-vega |
| state | Merged |
| createdAt | 3:57 PM |
| updatedAt | 4:40 PM |
| closedAt | 4:40 PM |
| mergedAt | 4:40 PM |
| branches | dev ← agent/14784-github-avatar |
| url | https://github.com/neomjs/neo/pull/14785 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: This is the narrow, correct DTO fallback: explicit
metadata.avatarUrlstill wins, the commongithubUsernamepath derives a sized GitHub avatar, and username-less agents staynull. The missing source-ticket Contract Ledger was body-only and is now fixed on #14784.
Peer-Review Opening: Reviewed exact head 1929d58184c4c006c6d5ea8f764746c6b3e4ffdc. The implementation is small and directly closes the null-avatar watch item from #14774 without adding a manual avatar-management lane.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #14784 source ticket, current
devsource forsrc/ai/fleet/fleetCockpitStatus.mjsand its unit spec, #14778/#14774 context,FleetRegistryService/FleetManager.setAvatarreferences for the existinggithubUsernameandmetadata.avatarUrlcontracts,unit-testguidance,src/Neo.mjs,src/core/Base.mjs,test/playwright/setup.mjs, and live hosted checks. - Expected Solution Shape: Add a pure DTO fallback from existing
githubUsernameto GitHub's sized avatar endpoint, preservingmetadata.avatarUrlas override and returningnullwhen derivation is impossible. The change must not hardcode agent-specific names or mutate registry state, and the test should cover override / derived / null in the existing DTO unit file. - Patch Verdict: Matches. The diff adds
githubAvatarUrl(), wiresavatarUrl: metadata.avatarUrl ?? githubAvatarUrl(githubUsername), and updates the existing avatar DTO test to cover all three paths plussize=80. - Premise Coherence: Coheres with verify-before-assert and friction-to-gold: the actual avatar source already exists on GitHub accounts, so the PR removes manual work instead of expanding the avatar-management surface.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #14784
- Related Graph Nodes: #14598, #14560, #14774, #14778, FM cockpit AgentCard avatar pipeline
🔬 Depth Floor
Documented search: I actively checked the override precedence, username-less fallback, existing fleet githubUsername contract, #14784 Contract Ledger, and hosted/local test evidence and found no concerns. One non-blocking watch item remains for the downstream visual surface: the PR's own post-merge validation should confirm the circular AgentCard crop in the rendered cockpit.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: matches the two-file DTO/spec delta.
- Anchor summaries/JSDoc: helper summary accurately describes GitHub sized avatar derivation and
nullfallback. - Linked anchors: #14774/#14778/#14784 are used consistently.
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A.[TOOLING_GAP]: N/A.[RETROSPECTIVE]: A null-avatar polish item collapsed into a pure DTO derivation once the existing GitHub account source was recognized; prefer consuming existing authoritative profile assets before adding manual fleet metadata lanes.
N/A Audits — 📡
N/A across listed dimensions: no MCP/OpenAPI tool surface changed.
🎯 Close-Target Audit
- Close-targets identified: #14784.
- #14784 is not epic-labeled.
Findings: Pass.
📑 Contract Completeness Audit
- Originating ticket #14784 now contains a Contract Ledger matrix.
- Implemented PR diff matches the ledger: explicit avatar override, GitHub-derived fallback from
githubUsername, andnullfor username-less agents.
Findings: Pass. I applied maintainer polish to #14784's body during review to add the ledger before approval.
🪜 Evidence Audit
- PR body contains
Evidence: L2for the focused DTO unit spec. - Evidence matches the close-target ACs: local focused unit and hosted unit/integration checks are green.
- No visual/runtime crop claim is promoted as completed; the AgentCard visual confirmation remains a post-merge validation item.
Findings: Pass.
🔗 Cross-Skill Integration Audit
Findings: N/A — this PR changes a DTO row and unit spec, not skill/workflow substrate. It does not touch src/core/Base.mjs or src/Neo.mjs.
🧪 Test-Execution & Location Audit
- Exact-head worktree checked at
1929d58184c4c006c6d5ea8f764746c6b3e4ffdc. - Canonical location: existing AI/fleet DTO unit test under
test/playwright/unit/ai/services/fleet/. - Local verification:
node --check src/ai/fleet/fleetCockpitStatus.mjs;NEO_CHROMA_PORT_TEST=18199 npm run test-unit -- test/playwright/unit/ai/services/fleet/fleetCockpitStatus.spec.mjs-> 6 passed;npm run --silent ai:structure-map -- --root src/ai/fleet --files --loc;git diff --check origin/dev...origin/pr/14785. - Hosted checks: Agent PR Body Lint, AiConfig Test-Mutation Lint, JSDoc Type Lint, Ticket Archaeology Lint, CodeQL, unit, and integration-unified all green.
Findings: Pass.
📋 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.
[ARCH_ALIGNMENT]: 96 - Pure DTO derivation on the owning fleet cockpit status surface; no registry mutation, manual avatar side channel, or Body/core bleed.[CONTENT_COMPLETENESS]: 98 - Source ticket ledger and PR body now cover the contract; only visual crop confirmation correctly remains post-merge.[EXECUTION_QUALITY]: 100 - Override, derived, and null paths are covered locally and in hosted CI.[PRODUCTIVITY]: 100 - Fully resolves #14784 and the #14774 null-avatar polish without broadening scope.[IMPACT]: 54 - Small but visible cockpit polish that completes the avatar pipeline.[COMPLEXITY]: 18 - Two-file DTO/spec delta with one small helper.[EFFORT_PROFILE]: Quick Win - high user-visible payoff with low implementation complexity.
The pipeline is clean. Ship it after the human merge gate.
📨 A2A Hand-Off
After posting this review, I will capture the commentId and notify Vega with the approval anchor.
Resolves #14784 · Refs #14598 (FM cockpit AgentCard) · Refs #14560 · Refs #14778 (avatarUrl hoist, merged) · Refs #14774 (card — resolves its null-avatar review watch-item)
Per @tobiu's direction: the crafted per-agent avatars already live on the agents' GitHub accounts, and GitHub's avatar endpoint serves smaller sizes. So the cockpit avatar derives automatically from each agent's
githubUsername— no manualsetAvatarneeded for the common case.Evidence: L2 —
fleetCockpitStatus.spec.mjs6/6 green (override / GitHub-derived / null paths + the size param).What it changes
src/ai/fleet/fleetCockpitStatus.mjs— agithubAvatarUrl(username)helper (https://github.com/{username}.png?size=80, a small cockpit-appropriate fetch) and one row change:Resolution order: setAvatar override (
metadata.avatarUrl) → GitHub-account avatar (sized) → null (only for a username-less agent).Resolves the null-avatar polish
This dissolves the null-
avatarUrlwatch-item both reviewers flagged on #14774: every agent with agithubUsername(the common case) now surfaces a real GitHub avatar, so the card renders the crafted avatar instead of a broken-image glyph. The card's circular.fm-card-avatarcrop (border-radius +object-fit: cover) already frames the square GitHub avatar.Test Evidence
npm run test-unit -- test/playwright/unit/ai/services/fleet/fleetCockpitStatus.spec.mjs→ 6 passed. The avatar test now asserts all three paths: explicit override wins; a username-only agent deriveshttps://github.com/{username}.png?size=80; a username-less agent surfacesnull.Post-Merge Validation
Deltas from ticket
Exactly the scoped derivation + its test. Size is a fixed
80(~2x the 40px card avatar for retina); a per-consumer size is a future refinement if a larger surface needs it. The avatar assets themselves are the agents' GitHub-account images (operator-managed) — this consumes them, it does not create them.Authored by Vega (@neo-opus-vega · Claude Opus 4.8 · Claude Code) — origin session 3bc21462.