From e0e36e970f7a2346052049ad8415fb24c98d91aa Mon Sep 17 00:00:00 2001 From: "JeongMinseok(Nalo)" Date: Tue, 19 May 2026 17:56:25 +0900 Subject: [PATCH 01/16] [issues-1] Hide completed agents from village by default + TopBar toggle (#4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(ui): hide completed agents from village by default, add TopBar toggle (#1) Server providers now auto-discover every Claude/Codex session jsonl on disk, so the village would slowly fill with old completed heroes that hadn't been touched in days. Default behavior is now to keep the village focused on agents that are still doing something; a TopBar toggle reveals completed heroes again for users who want the historical view. Single source of presentation policy lives in `agentPresentation.ts` — both the Party Bar and the Phaser scene consume the same derived list, and source-badge mixed-mode is computed by a shared helper so React and Phaser can't drift. TopBar stats / DetailPanel lookup / Activity Feed keep working off the unfiltered snapshot. Also covers the surrounding consistency work that fell out of the review: BuildingInfoPanel now consumes the same projection, the selection auto-clears when the toggle hides the currently-selected hero, and the existing night/rain toggles pick up `type="button"` + `aria-pressed` + `aria-label` to match the new toggle's a11y. * refactor(ui): keep waiting heroes visible + move presentation policy out of hooks/ (#1) Two issues surfaced by Codex review of the initial PR: - waiting was getting filtered out as a side effect of the new projection. The Phaser scene used to render waiting heroes (HeroSprite has a dedicated pulse/label), and computeShowSourceBadge still counts waiting as live, so the scene was losing turn-end heroes and the source badge could disagree with what was actually on screen. Treat waiting like active/idle in the projection and add a regression test. - agentPresentation.ts lived under hooks/, which made VillageScene (a Phaser adapter) import from the React hooks tree. The helper itself is the right abstraction — move it to client/src/presentation/ so both App.tsx and VillageScene can depend on it without crossing into React-specific land. --- client/src/App.tsx | 47 +++++-- client/src/components/BuildingInfoPanel.tsx | 7 +- client/src/components/PartyBar.tsx | 18 ++- client/src/components/TopBar.tsx | 33 ++++- client/src/game/scenes/VillageScene.ts | 20 +-- client/src/hooks/useTopBarPrefs.test.ts | 38 ++++++ client/src/hooks/useTopBarPrefs.ts | 77 +++++++++++ .../presentation/agentPresentation.test.ts | 127 ++++++++++++++++++ client/src/presentation/agentPresentation.ts | 58 ++++++++ 9 files changed, 396 insertions(+), 29 deletions(-) create mode 100644 client/src/hooks/useTopBarPrefs.test.ts create mode 100644 client/src/hooks/useTopBarPrefs.ts create mode 100644 client/src/presentation/agentPresentation.test.ts create mode 100644 client/src/presentation/agentPresentation.ts diff --git a/client/src/App.tsx b/client/src/App.tsx index 4264c1b..e4e9773 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -1,7 +1,9 @@ -import { useEffect, useState, useCallback } from 'react'; +import { useEffect, useMemo, useState, useCallback } from 'react'; import { PhaserGame } from './game/PhaserGame'; import { useAgentState } from './hooks/useAgentState'; import { useSelectedAgent } from './hooks/useSelectedAgent'; +import { useTopBarPrefs } from './hooks/useTopBarPrefs'; +import { computeShowSourceBadge, filterAgentsForPresentation } from './presentation/agentPresentation'; import { eventBridge } from './game/EventBridge'; import { TopBar } from './components/TopBar'; import { PartyBar } from './components/PartyBar'; @@ -15,6 +17,15 @@ import './App.css'; export default function App() { const { agents, activityLog, connected, configDirs } = useAgentState(); const { selectedAgentId, selectAgent } = useSelectedAgent(); + const [topBarPrefs, updateTopBarPrefs] = useTopBarPrefs(); + + // Presentation projection: agents shown in the Party Bar and the Phaser + // village scene. TopBar stats, DetailPanel lookup, ActivityFeed, and + // BuildingInfoPanel continue to use the unfiltered `agents` snapshot. + const presentationAgents = useMemo( + () => filterAgentsForPresentation(agents, topBarPrefs.showCompletedAgents), + [agents, topBarPrefs.showCompletedAgents], + ); const [selectedBuilding, setSelectedBuilding] = useState<{ id: string; anchor: { x: number; y: number }; @@ -33,10 +44,9 @@ export default function App() { // Only show source badges when both providers have a LIVE agent — completed // / error sessions don't count, otherwise the badge would linger after the - // last Codex hero finishes just because it's still in state. - const liveAgents = agents.filter((a) => a.status !== 'completed' && a.status !== 'error'); - const showSourceBadge = liveAgents.some((a) => a.source === 'claude') - && liveAgents.some((a) => a.source === 'codex'); + // last Codex hero finishes just because it's still in state. Computed by a + // shared helper so VillageScene stays in lockstep. + const showSourceBadge = useMemo(() => computeShowSourceBadge(agents), [agents]); // When selecting agent, clear building const handleSelectAgent = useCallback((id: string | null) => { @@ -103,8 +113,18 @@ export default function App() { }, []); useEffect(() => { - eventBridge.emit('agents:updated', agents); - }, [agents]); + eventBridge.emit('agents:updated', presentationAgents); + }, [presentationAgents]); + + // Deselect when the selected agent gets hidden by the presentation projection + // (e.g. user flips the "Show completed agents" toggle off while a completed + // hero is selected). Without this the DetailPanel would linger pointing at a + // hero that is no longer on screen. + useEffect(() => { + if (selectedAgentId === null) return; + const stillVisible = presentationAgents.some((a) => a.id === selectedAgentId); + if (!stillVisible) selectAgent(null); + }, [presentationAgents, selectedAgentId, selectAgent]); useEffect(() => { eventBridge.emit('selection:changed', selectedAgentId); @@ -123,10 +143,17 @@ export default function App() { {villageReady && (
- + + updateTopBarPrefs({ showCompletedAgents: !topBarPrefs.showCompletedAgents }) + } + /> setSelectedBuilding(null)} /> )} diff --git a/client/src/components/BuildingInfoPanel.tsx b/client/src/components/BuildingInfoPanel.tsx index f5a45c7..7853add 100644 --- a/client/src/components/BuildingInfoPanel.tsx +++ b/client/src/components/BuildingInfoPanel.tsx @@ -44,9 +44,10 @@ export function BuildingInfoPanel({ buildingId, anchor, agents, onClose }: Build if (building === undefined) return null; - const agentsHere = agents.filter( - (a) => a.currentActivity === building.activity && (a.status === 'active' || a.status === 'idle'), - ); + // `agents` is the App-level presentation projection — it already excludes + // `error` / `waiting`, and gates `completed` behind the TopBar toggle. + // We only need to match this building's activity here. + const agentsHere = agents.filter((a) => a.currentActivity === building.activity); const style = position === null ? { visibility: 'hidden' as const } diff --git a/client/src/components/PartyBar.tsx b/client/src/components/PartyBar.tsx index 4d5ebd9..5b7aa10 100644 --- a/client/src/components/PartyBar.tsx +++ b/client/src/components/PartyBar.tsx @@ -121,10 +121,13 @@ export function PartyBar({ agents, selectedAgentId, onSelectAgent, showSourceBad const [prefs, updatePrefs] = usePartyPrefs(); const mode: 'full' | 'icons' = prefs.foldState; - const visible = agents.filter((a) => a.status === 'active' || a.status === 'idle'); - const sorted = [...visible].sort((a, b) => STATUS_ORDER[a.status] - STATUS_ORDER[b.status]); - const activeCount = visible.filter((a) => a.status === 'active').length; - const idleCount = visible.filter((a) => a.status === 'idle').length; + // `agents` is the App-level presentation projection — it already excludes + // `error` / `waiting`, and only includes `completed` when the TopBar toggle + // is on. Sort by status (active first) so completed rows land at the bottom. + const sorted = [...agents].sort((a, b) => STATUS_ORDER[a.status] - STATUS_ORDER[b.status]); + const activeCount = agents.filter((a) => a.status === 'active').length; + const idleCount = agents.filter((a) => a.status === 'idle').length; + const completedCount = agents.filter((a) => a.status === 'completed').length; const toggleFold = useCallback(() => { updatePrefs({ foldState: mode === 'full' ? 'icons' : 'full' }); @@ -138,9 +141,12 @@ export function PartyBar({ agents, selectedAgentId, onSelectAgent, showSourceBad
{mode === 'full' ? ( - Party ({activeCount} active, {idleCount} idle) + + Party ({activeCount} active, {idleCount} idle + {completedCount > 0 ? `, ${completedCount} done` : ''}) + ) : ( - {activeCount + idleCount} + {sorted.length} )} + 2h + // App-level presentation projection already excludes `waiting` / `error`, + // and gates `completed` behind the TopBar toggle. The scene applies two + // additional local policies: hide `error` defensively, and drop heroes + // that have been idle for longer than IDLE_HIDE_THRESHOLD_MS so the + // village doesn't grow unbounded over a long-lived session. const visible = agents.filter((a) => { - if (a.status === 'completed' || a.status === 'error') return false; + if (a.status === 'error') return false; if (a.status === 'idle' && now - a.lastEvent > IDLE_HIDE_THRESHOLD_MS) return false; return true; }); // Mixed-provider mode: show source badges only when both Claude and Codex - // have a LIVE hero. Completed/error sessions don't count, so the badge - // disappears the moment the last non-dormant Codex hero finishes. Mirrors - // the flag computed in App.tsx — keep these two in sync. - const liveAgents = agents.filter((a) => a.status !== 'completed' && a.status !== 'error'); - const hasClaude = liveAgents.some((a) => a.source === 'claude'); - const hasCodex = liveAgents.some((a) => a.source === 'codex'); - const showSourceBadge = hasClaude && hasCodex; + // have a LIVE hero. Computed by the shared `computeShowSourceBadge` helper + // so this stays in lockstep with App.tsx — both call the same function + // against the unfiltered `agents` snapshot. + const showSourceBadge = computeShowSourceBadge(agents); // Remove heroes no longer visible for (const [id, hero] of this.heroes) { diff --git a/client/src/hooks/useTopBarPrefs.test.ts b/client/src/hooks/useTopBarPrefs.test.ts new file mode 100644 index 0000000..7da42e5 --- /dev/null +++ b/client/src/hooks/useTopBarPrefs.test.ts @@ -0,0 +1,38 @@ +import { describe, it, expect } from 'bun:test'; +import { DEFAULT_TOPBAR_PREFS, parseTopBarPrefs, mergeTopBarPrefs } from './useTopBarPrefs'; + +describe('parseTopBarPrefs', () => { + it('returns DEFAULT_TOPBAR_PREFS for null input', () => { + expect(parseTopBarPrefs(null)).toEqual(DEFAULT_TOPBAR_PREFS); + }); + + it('returns DEFAULT_TOPBAR_PREFS for malformed JSON', () => { + expect(parseTopBarPrefs('not json {')).toEqual(DEFAULT_TOPBAR_PREFS); + }); + + it('returns DEFAULT_TOPBAR_PREFS for non-object payload', () => { + expect(parseTopBarPrefs('"a string"')).toEqual(DEFAULT_TOPBAR_PREFS); + expect(parseTopBarPrefs('null')).toEqual(DEFAULT_TOPBAR_PREFS); + }); + + it('parses a valid payload', () => { + const raw = JSON.stringify({ showCompletedAgents: true }); + expect(parseTopBarPrefs(raw)).toEqual({ showCompletedAgents: true }); + }); + + it('falls back to default when showCompletedAgents has wrong type', () => { + const raw = JSON.stringify({ showCompletedAgents: 'yes' }); + expect(parseTopBarPrefs(raw)).toEqual(DEFAULT_TOPBAR_PREFS); + }); + + it('default has showCompletedAgents false (current behavior preserved)', () => { + expect(DEFAULT_TOPBAR_PREFS.showCompletedAgents).toBe(false); + }); +}); + +describe('mergeTopBarPrefs', () => { + it('overlays partial onto base', () => { + const merged = mergeTopBarPrefs(DEFAULT_TOPBAR_PREFS, { showCompletedAgents: true }); + expect(merged.showCompletedAgents).toBe(true); + }); +}); diff --git a/client/src/hooks/useTopBarPrefs.ts b/client/src/hooks/useTopBarPrefs.ts new file mode 100644 index 0000000..5302437 --- /dev/null +++ b/client/src/hooks/useTopBarPrefs.ts @@ -0,0 +1,77 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; + +export interface TopBarPrefs { + showCompletedAgents: boolean; +} + +export const DEFAULT_TOPBAR_PREFS: TopBarPrefs = { + showCompletedAgents: false, +}; + +const STORAGE_KEY = 'agentquest:topbar:prefs'; +const WRITE_DEBOUNCE_MS = 200; + +export function parseTopBarPrefs(raw: string | null): TopBarPrefs { + if (raw === null) return DEFAULT_TOPBAR_PREFS; + let obj: unknown; + try { obj = JSON.parse(raw); } catch { return DEFAULT_TOPBAR_PREFS; } + if (obj === null || typeof obj !== 'object') return DEFAULT_TOPBAR_PREFS; + const o = obj as Record; + return { + showCompletedAgents: + typeof o.showCompletedAgents === 'boolean' + ? o.showCompletedAgents + : DEFAULT_TOPBAR_PREFS.showCompletedAgents, + }; +} + +export function mergeTopBarPrefs(base: TopBarPrefs, patch: Partial): TopBarPrefs { + return { ...base, ...patch }; +} + +export function useTopBarPrefs(): [TopBarPrefs, (patch: Partial) => void] { + const [prefs, setPrefs] = useState(() => { + if (typeof window === 'undefined') return DEFAULT_TOPBAR_PREFS; + return parseTopBarPrefs(window.localStorage.getItem(STORAGE_KEY)); + }); + + const writeTimer = useRef | null>(null); + const pendingValue = useRef(null); + + // Two effects on purpose: the dep-cleanup of the first only cancels the + // pending timer so the debounce can keep coalescing rapid changes, while + // the unmount-cleanup of the second drains the pending value to disk. + // Merging them into one would flush on every prefs change and defeat the + // debounce. + + useEffect(() => { + if (writeTimer.current !== null) clearTimeout(writeTimer.current); + pendingValue.current = prefs; + writeTimer.current = setTimeout(() => { + try { + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(prefs)); + } catch { /* quota or private mode — silently ignore */ } + pendingValue.current = null; + }, WRITE_DEBOUNCE_MS); + return () => { + if (writeTimer.current !== null) clearTimeout(writeTimer.current); + }; + }, [prefs]); + + useEffect(() => { + return () => { + if (pendingValue.current !== null) { + try { + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(pendingValue.current)); + } catch { /* quota or private mode — silently ignore */ } + pendingValue.current = null; + } + }; + }, []); + + const update = useCallback((patch: Partial) => { + setPrefs((prev) => mergeTopBarPrefs(prev, patch)); + }, []); + + return [prefs, update]; +} diff --git a/client/src/presentation/agentPresentation.test.ts b/client/src/presentation/agentPresentation.test.ts new file mode 100644 index 0000000..14c7f54 --- /dev/null +++ b/client/src/presentation/agentPresentation.test.ts @@ -0,0 +1,127 @@ +import { describe, it, expect } from 'bun:test'; +import type { AgentState } from '../types/agent'; +import { computeShowSourceBadge, filterAgentsForPresentation } from './agentPresentation'; + +function makeAgent(overrides: Partial): AgentState { + const base: AgentState = { + id: 'a1', + name: 'Hero', + heroClass: 'warrior', + heroColor: 'blue', + status: 'active', + currentActivity: 'idle', + tokenUsage: { input: 0, output: 0, cacheRead: 0 }, + cost: 0, + sessionStart: 0, + toolCalls: [], + errors: [], + filesModified: [], + lastEvent: 0, + cwd: '/tmp', + configDir: '~/.claude', + source: 'claude', + } as AgentState; + return { ...base, ...overrides }; +} + +describe('filterAgentsForPresentation', () => { + it('default OFF: active, idle, and waiting pass; completed and error are hidden', () => { + const agents: AgentState[] = [ + makeAgent({ id: 'active1', status: 'active' }), + makeAgent({ id: 'idle1', status: 'idle' }), + makeAgent({ id: 'wait1', status: 'waiting' }), + makeAgent({ id: 'done1', status: 'completed' }), + makeAgent({ id: 'err1', status: 'error' }), + ]; + const visible = filterAgentsForPresentation(agents, false); + expect(visible.map((a) => a.id).sort()).toEqual(['active1', 'idle1', 'wait1']); + }); + + it('toggle ON: active, idle, waiting, and completed all pass', () => { + const agents: AgentState[] = [ + makeAgent({ id: 'active1', status: 'active' }), + makeAgent({ id: 'idle1', status: 'idle' }), + makeAgent({ id: 'wait1', status: 'waiting' }), + makeAgent({ id: 'done1', status: 'completed' }), + ]; + const visible = filterAgentsForPresentation(agents, true); + expect(visible.map((a) => a.id).sort()).toEqual(['active1', 'done1', 'idle1', 'wait1']); + }); + + it('toggle ON: error remains hidden', () => { + const agents: AgentState[] = [ + makeAgent({ id: 'err1', status: 'error' }), + makeAgent({ id: 'done1', status: 'completed' }), + ]; + const visible = filterAgentsForPresentation(agents, true); + expect(visible.map((a) => a.id).sort()).toEqual(['done1']); + }); + + it('waiting passes regardless of the completed toggle (regression for turn-end heroes)', () => { + const agents: AgentState[] = [makeAgent({ id: 'turnend', status: 'waiting' })]; + expect(filterAgentsForPresentation(agents, false).map((a) => a.id)).toEqual(['turnend']); + expect(filterAgentsForPresentation(agents, true).map((a) => a.id)).toEqual(['turnend']); + }); + + it('preserves all idle and active agents regardless of toggle', () => { + const agents: AgentState[] = [ + makeAgent({ id: 'a', status: 'active' }), + makeAgent({ id: 'b', status: 'idle' }), + makeAgent({ id: 'c', status: 'active' }), + ]; + expect(filterAgentsForPresentation(agents, false).length).toBe(3); + expect(filterAgentsForPresentation(agents, true).length).toBe(3); + }); + + it('returns empty array for empty input', () => { + expect(filterAgentsForPresentation([], false)).toEqual([]); + expect(filterAgentsForPresentation([], true)).toEqual([]); + }); +}); + +describe('computeShowSourceBadge', () => { + it('returns false when no agents', () => { + expect(computeShowSourceBadge([])).toBe(false); + }); + + it('returns false when only one provider has live agents', () => { + const agents: AgentState[] = [ + makeAgent({ id: 'c1', source: 'claude', status: 'active' }), + makeAgent({ id: 'c2', source: 'claude', status: 'idle' }), + ]; + expect(computeShowSourceBadge(agents)).toBe(false); + }); + + it('returns true when both providers have at least one live agent', () => { + const agents: AgentState[] = [ + makeAgent({ id: 'c1', source: 'claude', status: 'active' }), + makeAgent({ id: 'x1', source: 'codex', status: 'idle' }), + ]; + expect(computeShowSourceBadge(agents)).toBe(true); + }); + + it('ignores completed and error agents when computing liveness', () => { + const agents: AgentState[] = [ + makeAgent({ id: 'c1', source: 'claude', status: 'active' }), + makeAgent({ id: 'x1', source: 'codex', status: 'completed' }), + makeAgent({ id: 'x2', source: 'codex', status: 'error' }), + ]; + expect(computeShowSourceBadge(agents)).toBe(false); + }); + + it('handles a single live codex agent alongside a completed claude agent', () => { + const agents: AgentState[] = [ + makeAgent({ id: 'c1', source: 'claude', status: 'completed' }), + makeAgent({ id: 'x1', source: 'codex', status: 'active' }), + ]; + expect(computeShowSourceBadge(agents)).toBe(false); + }); + + it('counts waiting agents as live (consistent with filterAgentsForPresentation)', () => { + const agents: AgentState[] = [ + makeAgent({ id: 'c1', source: 'claude', status: 'waiting' }), + makeAgent({ id: 'x1', source: 'codex', status: 'active' }), + ]; + expect(computeShowSourceBadge(agents)).toBe(true); + }); +}); diff --git a/client/src/presentation/agentPresentation.ts b/client/src/presentation/agentPresentation.ts new file mode 100644 index 0000000..9a57715 --- /dev/null +++ b/client/src/presentation/agentPresentation.ts @@ -0,0 +1,58 @@ +import type { AgentState } from '../types/agent'; + +/** + * Project the full `agents` snapshot to the subset displayed in the Party + * Bar and the Phaser village scene. TopBar stats and DetailPanel lookup + * keep using the unfiltered list — this projection only governs the two + * presentation surfaces. + * + * This function is the single authoritative gate for which statuses reach + * those surfaces: + * + * - `active`, `idle`, `waiting` — always pass through. `waiting` is a + * short-lived "turn just ended, hero is resting at the tavern" state; + * HeroSprite has dedicated pulse/label rendering for it, so it must + * reach the Phaser scene. computeShowSourceBadge treats waiting as + * live too — keep the two functions in agreement. + * - `completed` — pass through only when `showCompleted` is true. + * - `error` — always blocked here. The scene additionally drops `error` + * defensively, and there is currently no Party Bar surfacing for it. + * + * Downstream consumers may apply additional local hiding (e.g. the Phaser + * scene drops heroes whose last event is older than its idle threshold), + * but they MUST NOT re-introduce `error` once it has been filtered out. + */ +export function filterAgentsForPresentation( + agents: AgentState[], + showCompleted: boolean, +): AgentState[] { + return agents.filter((agent) => { + if (agent.status === 'active' || agent.status === 'idle' || agent.status === 'waiting') { + return true; + } + if (agent.status === 'completed' && showCompleted) return true; + return false; + }); +} + +/** + * Whether the source-provider badge (Claude/Codex orange/teal pill) should + * be shown anywhere in the UI. Only true when both providers have at least + * one LIVE agent — completed/error sessions don't count, otherwise the + * badge would linger after the last Codex hero finishes. + * + * Used by both App.tsx (React overlays) and VillageScene.ts (Phaser sprite + * badges); keep them in sync by calling this function instead of inlining + * the logic. + */ +export function computeShowSourceBadge(agents: AgentState[]): boolean { + let hasClaude = false; + let hasCodex = false; + for (const a of agents) { + if (a.status === 'completed' || a.status === 'error') continue; + if (a.source === 'claude') hasClaude = true; + else if (a.source === 'codex') hasCodex = true; + if (hasClaude && hasCodex) return true; + } + return false; +} From b06049c5510236a4706d2d8d2ccfebd9ab5511da Mon Sep 17 00:00:00 2001 From: "JeongMinseok(Nalo)" Date: Tue, 19 May 2026 18:02:08 +0900 Subject: [PATCH 02/16] [issues-2] feat(agents): mirror Claude Code session display title above hero sprites (#5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(agents): mirror Claude Code session display title above hero sprites Server-side: SessionRegistry now reads the `jobId` out of every `/sessions/.json` it scans and resolves the matching `/jobs//state.json` for the user-set session title (the same string Claude Code's `claude agents` view renders in the left column). The title is exposed through a new SessionDisplayNameOracle that AgentStateManager consults on createAgent / processEvent / refreshAll, overriding the slug/cwd fallback whenever a display name is present. Codex sessions and subagents (no jobId of their own) keep their existing labels. Client-side: HeroSprite gains updateName(name) for live retitle support, and every head-stack label now flows through a code-point-safe truncateLabel helper so long titles ellipsize cleanly without breaking emoji surrogate pairs. VillageScene's hero update loop calls updateName each tick (idempotent guard prevents redundant DOM writes), and EditorTopBar's slot-name truncation switches to the same helper for consistency. Closes #2 * refactor: address Codex senior + architect review comments - Restore the derived (slug/cwd) name when the display-name oracle drops the user-set title. Without the fallback path a removed or corrupt `state.json` would freeze the previous label on the sprite forever. AgentState now carries `derivedName` so the manager can swap back without consulting the JSONL slug again. - Move `truncateLabel` from `client/src/game/entities/` to `client/src/utils/` — the helper is consumed by both the village game scene and the editor topbar, so its home belongs in the neutral utils layer instead of crossing the editor → game-entities boundary. - Tighten the `SessionDisplayNameOracle` interface contract so it no longer leaks the concrete Claude storage paths into the high-level state policy; adapter-specific details live in `SessionRegistry`. - Add regression tests for the oracle-drop fallback (both refreshAll and processEvent paths). --- client/src/editor/panels/EditorTopBar.tsx | 3 +- client/src/game/entities/HeroSprite.ts | 31 +++-- client/src/game/scenes/VillageScene.ts | 1 + client/src/utils/truncateLabel.test.ts | 53 +++++++++ client/src/utils/truncateLabel.ts | 15 +++ server/src/index.ts | 1 + server/src/session-registry.test.ts | 114 +++++++++++++++++- server/src/session-registry.ts | 54 +++++++-- server/src/state/agent-state-manager.test.ts | 117 +++++++++++++++++++ server/src/state/agent-state-manager.ts | 60 ++++++++-- server/src/types.ts | 15 ++- 11 files changed, 436 insertions(+), 28 deletions(-) create mode 100644 client/src/utils/truncateLabel.test.ts create mode 100644 client/src/utils/truncateLabel.ts diff --git a/client/src/editor/panels/EditorTopBar.tsx b/client/src/editor/panels/EditorTopBar.tsx index e99b08e..c444a82 100644 --- a/client/src/editor/panels/EditorTopBar.tsx +++ b/client/src/editor/panels/EditorTopBar.tsx @@ -1,6 +1,7 @@ import { editorBridge } from '../EditorBridge'; import { editorStore, useEditorStore } from '../state/editor-store'; import type { SlotInfo } from '../types/map'; +import { truncateLabel } from '../../utils/truncateLabel'; export function EditorTopBar() { const dirty = useEditorStore((s) => s.dirty); @@ -57,7 +58,7 @@ export function EditorTopBar() { const getSlotLabel = (info: SlotInfo): string => { const star = info.isActive ? ' \u2605' : ''; if (info.isEmpty) return `${info.slot}${star}`; - const name = info.name.length > 10 ? info.name.slice(0, 10) + '...' : info.name; + const name = truncateLabel(info.name, 10); return `${info.slot} \u2014 ${name}${star}`; }; diff --git a/client/src/game/entities/HeroSprite.ts b/client/src/game/entities/HeroSprite.ts index d47fb9e..7d0d0bc 100644 --- a/client/src/game/entities/HeroSprite.ts +++ b/client/src/game/entities/HeroSprite.ts @@ -3,6 +3,7 @@ import { HERO_COLOR_SPRITE_BASE, HERO_LABEL_COLOR, SOURCE_BADGE_COLOR, modelBadg import { getActiveTheme } from '../themes/registry'; import { findRoadPath, type Point } from '../data/road-network'; import { addCrispText } from '../text'; +import { truncateLabel } from '../../utils/truncateLabel'; const MOVE_SPEED = 150; /** Ground distance covered by one full run-cycle. Keeps legs synced to travel. */ @@ -15,6 +16,13 @@ const RUN_PIXELS_PER_CYCLE = 60; * (sprite 96 px → name -50, activity +46, detail +60, task +74). */ const TASK_MAX_CHARS = 28; +/** + * Hero head labels mirror the user's `cc_session_step` output — + * `[#1234, 12/13] some long description`. Cap roughly matches Claude Code's + * own `agents` view column so the label stays readable above the sprite + * without bleeding into adjacent heroes. + */ +const NAME_MAX_CHARS = 40; const ACTIVITY_COLOR: Record = { idle: '#888888', @@ -182,7 +190,7 @@ export class HeroSprite { const nameColor = HERO_LABEL_COLOR[heroColor] ?? '#DDDDDD'; this.nameBaseColor = nameColor; - this.nameText = addCrispText(scene, x, y + this.nameOffsetY, name, { + this.nameText = addCrispText(scene, x, y + this.nameOffsetY, truncateLabel(name, NAME_MAX_CHARS), { fontSize: '14px', color: nameColor, fontFamily: 'monospace', @@ -522,17 +530,26 @@ export class HeroSprite { this.modelText.setPosition(leftEdge + widthA + gap, y); } + /** + * Update the name label above the hero. Truncates with an ellipsis when the + * label exceeds NAME_MAX_CHARS so the rest of the head stack (subagent + * marker, activity, detail, task) stays inside the hero's visual footprint. + * No-op when `name` matches what's already rendered (avoids touching the + * Phaser text object on every WebSocket tick). + */ + updateName(name: string): void { + const truncated = truncateLabel(name, NAME_MAX_CHARS); + if (this.nameText.text === truncated) return; + this.nameText.setText(truncated); + } + /** Update the truncated task line shown below the detail. */ updateTask(task?: string): void { if (task === undefined || task.length === 0) { this.taskText.setText(''); return; } - const single = task.replace(/\s+/g, ' ').trim(); - const text = single.length > TASK_MAX_CHARS - ? single.slice(0, TASK_MAX_CHARS - 1) + '\u2026' - : single; - this.taskText.setText(text); + this.taskText.setText(truncateLabel(task, TASK_MAX_CHARS)); } /** Update the detail line shown below the activity label. */ @@ -543,7 +560,7 @@ export class HeroSprite { const parts = file.split('/'); detail = parts[parts.length - 1] ?? file; } else if (command) { - detail = command.length > 25 ? command.slice(0, 24) + '\u2026' : command; + detail = truncateLabel(command, 25); } this.detailText.setText(detail); } diff --git a/client/src/game/scenes/VillageScene.ts b/client/src/game/scenes/VillageScene.ts index d6b28f7..52e8dc7 100644 --- a/client/src/game/scenes/VillageScene.ts +++ b/client/src/game/scenes/VillageScene.ts @@ -699,6 +699,7 @@ export class VillageScene extends Phaser.Scene { buildingsToReposition.add(buildingDef.id); } else { // Always update detail text (file/command changes even without activity change) + existing.updateName(agent.name); existing.updateDetail(agent.currentFile, agent.currentCommand); existing.updateTask(agent.currentTask); existing.setStatus(agent.status); diff --git a/client/src/utils/truncateLabel.test.ts b/client/src/utils/truncateLabel.test.ts new file mode 100644 index 0000000..bcf03e6 --- /dev/null +++ b/client/src/utils/truncateLabel.test.ts @@ -0,0 +1,53 @@ +import { describe, test, expect } from 'bun:test'; + +import { truncateLabel } from './truncateLabel'; + +describe('truncateLabel', () => { + test('returns the input untouched when shorter than the limit', () => { + expect(truncateLabel('short', 40)).toBe('short'); + }); + + test('returns the input untouched when exactly the limit', () => { + const exact = 'x'.repeat(40); + expect(truncateLabel(exact, 40)).toBe(exact); + }); + + test('clips with an ellipsis when longer than the limit', () => { + const long = 'x'.repeat(50); + const out = truncateLabel(long, 10); + expect(out).toBe('xxxxxxxxx…'); + expect([...out].length).toBe(10); + }); + + test('collapses whitespace runs to single spaces', () => { + expect(truncateLabel('feat:\n fix\t\tcrash', 40)).toBe('feat: fix crash'); + }); + + test('trims leading and trailing whitespace before measuring length', () => { + // Without trim, the input has 8 chars and would not be truncated at 7; + // after trim it's 5 chars and stays intact. + expect(truncateLabel(' foo ', 7)).toBe('foo'); + }); + + test('does not slice an emoji in half (code-point safe)', () => { + // Each rocket is one user-perceived character but two UTF-16 code units. + // A naive `slice` would cut the second rocket mid-surrogate-pair and the + // canvas would render the U+FFFD replacement glyph. + const input = '🚀🚀🚀🚀🚀'; + const out = truncateLabel(input, 3); + expect(out).toBe('🚀🚀…'); + // The ellipsis is one code point — total three "characters". + expect([...out].length).toBe(3); + }); + + test('handles empty input', () => { + expect(truncateLabel('', 40)).toBe(''); + expect(truncateLabel(' ', 40)).toBe(''); + }); + + test('handles a max of 1 (degenerate but legal)', () => { + // After trim the single remaining char fits; ellipsis only kicks in past the cap. + expect(truncateLabel('a', 1)).toBe('a'); + expect(truncateLabel('ab', 1)).toBe('…'); + }); +}); diff --git a/client/src/utils/truncateLabel.ts b/client/src/utils/truncateLabel.ts new file mode 100644 index 0000000..7ca064d --- /dev/null +++ b/client/src/utils/truncateLabel.ts @@ -0,0 +1,15 @@ +/** + * Collapse whitespace and clip to `max` chars — every head-stack label sits on + * a single line, so wrapped multi-line strings would push neighbouring badges + * out of frame. The ellipsis preserves intent ("there was more") without + * leaking formatting noise (newlines, tabs) into the canvas. Spread iterates + * code points (not UTF-16 code units), so emoji and other supplementary-plane + * characters don't get sliced mid-surrogate-pair and render as replacement + * glyphs above the hero. + */ +export function truncateLabel(text: string, max: number): string { + const single = text.replace(/\s+/g, ' ').trim(); + const chars = [...single]; + if (chars.length <= max) return single; + return chars.slice(0, max - 1).join('') + '…'; +} diff --git a/server/src/index.ts b/server/src/index.ts index 3a0c45d..a531f59 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -53,6 +53,7 @@ const stateManager = new AgentStateManager({ subagentCompletedThresholdMs: SUBAGENT_COMPLETED_THRESHOLD_MS, subagentBusyCompletedThresholdMs: SUBAGENT_BUSY_COMPLETED_THRESHOLD_MS, livenessOracle: sessionRegistry, + displayNameOracle: sessionRegistry, }); const wsServer = new WebSocketServer(); const mapStorage = new MapStorage(); diff --git a/server/src/session-registry.test.ts b/server/src/session-registry.test.ts index 35cb8b3..4dc1b45 100644 --- a/server/src/session-registry.test.ts +++ b/server/src/session-registry.test.ts @@ -5,13 +5,38 @@ import { join } from 'node:path'; import { SessionRegistry } from './session-registry'; -async function makeClaudeDir(withSessions: Array<{ pid: number; sessionId: string }>): Promise { +interface SessionStub { + pid: number; + sessionId: string; + /** When set, the stub also writes `/jobs//state.json` so the display-name oracle has something to read. */ + jobId?: string; + /** When set, the corresponding `jobs//state.json` carries this `name`. */ + displayName?: string; +} + +async function makeClaudeDir(withSessions: SessionStub[]): Promise { const root = await mkdtemp(join(tmpdir(), 'agent-quest-test-')); const sessionsDir = join(root, 'sessions'); await mkdir(sessionsDir, { recursive: true }); - for (const { pid, sessionId } of withSessions) { + for (const stub of withSessions) { + const { pid, sessionId, jobId, displayName } = stub; const file = join(sessionsDir, `${pid}.json`); - await writeFile(file, JSON.stringify({ pid, sessionId, cwd: '/tmp', startedAt: Date.now(), kind: 'interactive', entrypoint: 'cli' })); + const payload: Record = { + pid, + sessionId, + cwd: '/tmp', + startedAt: Date.now(), + kind: 'interactive', + entrypoint: 'cli', + }; + if (jobId !== undefined) payload['jobId'] = jobId; + await writeFile(file, JSON.stringify(payload)); + + if (jobId !== undefined && displayName !== undefined) { + const jobDir = join(root, 'jobs', jobId); + await mkdir(jobDir, { recursive: true }); + await writeFile(join(jobDir, 'state.json'), JSON.stringify({ name: displayName })); + } } return root; } @@ -118,6 +143,89 @@ describe('SessionRegistry', () => { expect(reg.hasAnyLive()).toBe(false); }); + test('exposes display name read from jobs//state.json', async () => { + const dir = await makeClaudeDir([ + { pid: 11, sessionId: 'with-job', jobId: 'job-abc', displayName: '[#42, 7/13] feat: foo' }, + { pid: 12, sessionId: 'no-job' }, + ]); + tempDirs.push(dir); + + const reg = new SessionRegistry({ configDirs: [dir], pidAlive: () => true }); + await reg.scan(); + + expect(reg.getDisplayName('with-job')).toBe('[#42, 7/13] feat: foo'); + // No jobId → no display name (caller falls back to slug/cwd). + expect(reg.getDisplayName('no-job')).toBeUndefined(); + // Unknown sessionId → undefined. + expect(reg.getDisplayName('never-seen')).toBeUndefined(); + }); + + test('forgets display name when state.json is removed between scans', async () => { + const dir = await makeClaudeDir([ + { pid: 21, sessionId: 'sid-21', jobId: 'job-21', displayName: '[#7, 2/13] initial' }, + ]); + tempDirs.push(dir); + + const reg = new SessionRegistry({ configDirs: [dir], pidAlive: () => true }); + await reg.scan(); + expect(reg.getDisplayName('sid-21')).toBe('[#7, 2/13] initial'); + + await rm(join(dir, 'jobs', 'job-21', 'state.json')); + await reg.scan(); + expect(reg.getDisplayName('sid-21')).toBeUndefined(); + }); + + test('picks up display name changes between scans (live retitle)', async () => { + const dir = await makeClaudeDir([ + { pid: 31, sessionId: 'sid-31', jobId: 'job-31', displayName: '[#9, 2/13] foo' }, + ]); + tempDirs.push(dir); + + const reg = new SessionRegistry({ configDirs: [dir], pidAlive: () => true }); + await reg.scan(); + expect(reg.getDisplayName('sid-31')).toBe('[#9, 2/13] foo'); + + await writeFile( + join(dir, 'jobs', 'job-31', 'state.json'), + JSON.stringify({ name: '[#9, 12/13] foo' }), + ); + await reg.scan(); + expect(reg.getDisplayName('sid-31')).toBe('[#9, 12/13] foo'); + }); + + test('tolerates corrupt state.json (returns undefined)', async () => { + const dir = await makeClaudeDir([ + { pid: 41, sessionId: 'sid-41', jobId: 'job-41' }, + ]); + tempDirs.push(dir); + // Write a malformed state.json manually. + const jobDir = join(dir, 'jobs', 'job-41'); + await mkdir(jobDir, { recursive: true }); + await writeFile(join(jobDir, 'state.json'), '{not json'); + + const reg = new SessionRegistry({ configDirs: [dir], pidAlive: () => true }); + await reg.scan(); + + // Liveness still works; only displayName degrades gracefully. + expect(reg.isLive('sid-41')).toBe(true); + expect(reg.getDisplayName('sid-41')).toBeUndefined(); + }); + + test('drops display name for sessions whose pid is dead', async () => { + const dir = await makeClaudeDir([ + { pid: 51, sessionId: 'sid-51', jobId: 'job-51', displayName: '[#1, 5/13] x' }, + ]); + tempDirs.push(dir); + + const reg = new SessionRegistry({ configDirs: [dir], pidAlive: () => false }); + await reg.scan(); + + // Dead pid: registry must not surface stale display names — otherwise the + // dashboard would keep ghost sessions visible under their last-known title. + expect(reg.isLive('sid-51')).toBe(false); + expect(reg.getDisplayName('sid-51')).toBeUndefined(); + }); + test('setConfigDirs refreshes the watched roots', async () => { const a = await makeClaudeDir([{ pid: 1, sessionId: 'in-a' }]); const b = await makeClaudeDir([{ pid: 2, sessionId: 'in-b' }]); diff --git a/server/src/session-registry.ts b/server/src/session-registry.ts index 02c3102..f88bac8 100644 --- a/server/src/session-registry.ts +++ b/server/src/session-registry.ts @@ -1,15 +1,16 @@ import { readdir } from 'node:fs/promises'; import { join } from 'node:path'; -import type { SessionLivenessOracle } from './state/agent-state-manager'; +import type { SessionDisplayNameOracle, SessionLivenessOracle } from './state/agent-state-manager'; -/** Reads `sessionId` out of a `.json` file. Tolerates noise and missing fields. */ -async function readSessionIdFrom(filePath: string): Promise { +/** Reads `sessionId` and `jobId` out of a `.json` file. Tolerates noise and missing fields. */ +async function readSessionMetaFrom(filePath: string): Promise<{ sessionId: string; jobId: string | null } | null> { try { const text = await Bun.file(filePath).text(); - const data = JSON.parse(text) as { sessionId?: unknown }; + const data = JSON.parse(text) as { sessionId?: unknown; jobId?: unknown }; if (typeof data.sessionId === 'string' && data.sessionId.length > 0) { - return data.sessionId; + const jobId = typeof data.jobId === 'string' && data.jobId.length > 0 ? data.jobId : null; + return { sessionId: data.sessionId, jobId }; } } catch { // unreadable / not JSON / partially-written — just ignore this file @@ -17,6 +18,25 @@ async function readSessionIdFrom(filePath: string): Promise { return null; } +/** + * Reads the human-friendly display name out of `/jobs//state.json`. + * This is the same `name` field that Claude Code's `claude agents` view renders in + * the left column — see `~/.claude/rules/session-display-name.md`. Returns null + * when the file is missing, unparseable, or omits the field. + */ +async function readDisplayNameFrom(filePath: string): Promise { + try { + const text = await Bun.file(filePath).text(); + const data = JSON.parse(text) as { name?: unknown }; + if (typeof data.name === 'string' && data.name.length > 0) { + return data.name; + } + } catch { + // missing / unreadable / not JSON — fall back to slug/cwd at the caller. + } + return null; +} + /** Dependency-injectable liveness check; real impl uses `process.kill(pid, 0)`. */ export type PidLivenessCheck = (pid: number) => boolean; @@ -44,10 +64,11 @@ export interface SessionRegistryOptions { * agents whose JSONLs were touched by Claude Code resume/hook machinery but * whose real process has long since exited. */ -export class SessionRegistry implements SessionLivenessOracle { +export class SessionRegistry implements SessionLivenessOracle, SessionDisplayNameOracle { private configDirs: string[]; private pidAlive: PidLivenessCheck; private liveSessionIds = new Set(); + private displayNames = new Map(); private scanned = false; private pollInterval: ReturnType | null = null; @@ -69,6 +90,15 @@ export class SessionRegistry implements SessionLivenessOracle { return this.liveSessionIds.has(sessionId); } + /** + * Returns the user-set display name for a live Claude session, or undefined + * when the session has no jobId, no state.json, or the file omits the field. + * Callers fall back to slug/cwd in that case. + */ + getDisplayName(sessionId: string): string | undefined { + return this.displayNames.get(sessionId); + } + /** Current live session IDs (copy, for debugging/snapshots). */ snapshot(): string[] { return [...this.liveSessionIds]; @@ -92,6 +122,7 @@ export class SessionRegistry implements SessionLivenessOracle { async scan(): Promise { const next = new Set(); + const nextNames = new Map(); for (const configDir of this.configDirs) { const sessionsDir = join(configDir, 'sessions'); let entries: string[]; @@ -106,11 +137,18 @@ export class SessionRegistry implements SessionLivenessOracle { const pid = Number.parseInt(pidMatch[1]!, 10); if (!Number.isFinite(pid) || pid <= 0) continue; if (!this.pidAlive(pid)) continue; - const sessionId = await readSessionIdFrom(join(sessionsDir, entry)); - if (sessionId !== null) next.add(sessionId); + const meta = await readSessionMetaFrom(join(sessionsDir, entry)); + if (meta === null) continue; + next.add(meta.sessionId); + if (meta.jobId !== null) { + const statePath = join(configDir, 'jobs', meta.jobId, 'state.json'); + const displayName = await readDisplayNameFrom(statePath); + if (displayName !== null) nextNames.set(meta.sessionId, displayName); + } } } this.liveSessionIds = next; + this.displayNames = nextNames; this.scanned = true; } } diff --git a/server/src/state/agent-state-manager.test.ts b/server/src/state/agent-state-manager.test.ts index aa08cbe..8005cd4 100644 --- a/server/src/state/agent-state-manager.test.ts +++ b/server/src/state/agent-state-manager.test.ts @@ -740,4 +740,121 @@ describe('AgentStateManager', () => { expect(res!.agent.source).toBe('codex'); expect(res!.agent.status).toBe('active'); // NOT completed despite oracle saying "not live" }); + + test('display-name oracle overrides slug on agent creation', () => { + const displayNames = new Map([['sess-1', '[#42, 7/13] feat: foo']]); + const m = new AgentStateManager({ + displayNameOracle: { getDisplayName: (sid) => displayNames.get(sid) }, + }); + + const result = m.processEvent(makeEvent()); + + expect(result!.agent.name).toBe('[#42, 7/13] feat: foo'); + }); + + test('display-name oracle falls back to slug when no name is set', () => { + const m = new AgentStateManager({ + displayNameOracle: { getDisplayName: () => undefined }, + }); + + const result = m.processEvent(makeEvent()); + + expect(result!.agent.name).toBe('bubbly-waddling-cat'); + }); + + test('display-name oracle updates the agent label on subsequent events', () => { + const displayNames = new Map(); + const m = new AgentStateManager({ + displayNameOracle: { getDisplayName: (sid) => displayNames.get(sid) }, + }); + + const initial = m.processEvent(makeEvent()); + expect(initial!.agent.name).toBe('bubbly-waddling-cat'); + + // User retitles the session mid-run (cc_session_step 12) — the next + // processEvent must surface the new label without waiting for refreshAll. + displayNames.set('sess-1', '[#42, 12/13] feat: foo'); + const next = m.processEvent(makeEvent({ timestamp: Date.now() + 1 })); + + expect(next!.agent.name).toBe('[#42, 12/13] feat: foo'); + }); + + test('refreshAll picks up display-name changes and reports them as changed', () => { + const displayNames = new Map(); + const m = new AgentStateManager({ + displayNameOracle: { getDisplayName: (sid) => displayNames.get(sid) }, + }); + m.processEvent(makeEvent()); + expect(m.getAgent('sess-1')!.name).toBe('bubbly-waddling-cat'); + + // No JSONL event arrived, but the registry's next scan tick discovered a + // new state.json. refreshAll must rebroadcast on the strength of the name + // change alone — otherwise the dashboard freezes on the stale slug. + displayNames.set('sess-1', '[#42, DONE] feat: foo'); + const changed = m.refreshAll(); + + expect(changed).toContain('sess-1'); + expect(m.getAgent('sess-1')!.name).toBe('[#42, DONE] feat: foo'); + }); + + test('display-name oracle does NOT relabel subagents (they have no jobId)', () => { + const m = new AgentStateManager({ + displayNameOracle: { + // Bug-bait: an oracle that returns a value for a subagent id would + // wipe the filename-derived label, so the manager must skip the lookup. + getDisplayName: () => '[#42, 7/13] feat: foo', + }, + }); + + const result = m.processEvent(makeEvent({ sessionId: 'agent-helper-1234567890abcdef' }), '', 'claude', 'helper'); + + expect(result!.agent.name).toBe('helper'); + }); + + test('falls back to the derived name when the oracle drops the display name', () => { + const displayNames = new Map([['sess-1', '[#42, 7/13] feat: foo']]); + const m = new AgentStateManager({ + displayNameOracle: { getDisplayName: (sid) => displayNames.get(sid) }, + }); + + // Oracle has a title — agent should pick it up. + m.processEvent(makeEvent()); + expect(m.getAgent('sess-1')!.name).toBe('[#42, 7/13] feat: foo'); + + // User removes the state.json (the registry's next scan would drop the entry). + // Without the fallback path, agent.name would freeze on the stale title and + // refreshAll() wouldn't even mark the session as changed. + displayNames.delete('sess-1'); + const changed = m.refreshAll(); + + expect(changed).toContain('sess-1'); + expect(m.getAgent('sess-1')!.name).toBe('bubbly-waddling-cat'); + }); + + test('falls back to the derived name on the next processEvent after oracle drops the title', () => { + const displayNames = new Map([['sess-1', '[#42, 7/13] feat: foo']]); + const m = new AgentStateManager({ + displayNameOracle: { getDisplayName: (sid) => displayNames.get(sid) }, + }); + + m.processEvent(makeEvent()); + expect(m.getAgent('sess-1')!.name).toBe('[#42, 7/13] feat: foo'); + + displayNames.delete('sess-1'); + const next = m.processEvent(makeEvent({ timestamp: Date.now() + 1 })); + + expect(next!.agent.name).toBe('bubbly-waddling-cat'); + }); + + test('setDisplayNameOracle wires the oracle after construction', () => { + const m = new AgentStateManager(); + m.processEvent(makeEvent()); + expect(m.getAgent('sess-1')!.name).toBe('bubbly-waddling-cat'); + + m.setDisplayNameOracle({ getDisplayName: () => '[#1, 5/13] late wiring' }); + const changed = m.refreshAll(); + + expect(changed).toContain('sess-1'); + expect(m.getAgent('sess-1')!.name).toBe('[#1, 5/13] late wiring'); + }); }); diff --git a/server/src/state/agent-state-manager.ts b/server/src/state/agent-state-manager.ts index 9f753af..3137ced 100644 --- a/server/src/state/agent-state-manager.ts +++ b/server/src/state/agent-state-manager.ts @@ -49,6 +49,16 @@ export interface SessionLivenessOracle { isLive(sessionId: string): boolean; } +/** + * Display-name oracle: returns an optional human-readable label for a session. + * Returns undefined when no label is available — callers fall back to the + * slug/cwd-basename derivation. The concrete storage (file paths, formats) is + * the adapter's concern, not part of this contract. + */ +export interface SessionDisplayNameOracle { + getDisplayName(sessionId: string): string | undefined; +} + export interface AgentStateManagerOptions { /** Age above which an agent transitions from active → idle (ms). Default 5 min. */ idleThresholdMs?: number; @@ -72,6 +82,8 @@ export interface AgentStateManagerOptions { subagentBusyCompletedThresholdMs?: number; /** Optional oracle for cross-referencing sessions against live Claude pids. */ livenessOracle?: SessionLivenessOracle; + /** Optional oracle for surfacing the user-set session display name as the agent label. */ + displayNameOracle?: SessionDisplayNameOracle; } function isSubagentSessionId(sessionId: string): boolean { @@ -89,6 +101,7 @@ export class AgentStateManager { private subagentCompletedThresholdMs: number; private subagentBusyCompletedThresholdMs: number; private livenessOracle: SessionLivenessOracle | undefined; + private displayNameOracle: SessionDisplayNameOracle | undefined; constructor(opts: AgentStateManagerOptions = {}) { this.idleThresholdMs = opts.idleThresholdMs ?? 5 * 60_000; @@ -98,12 +111,17 @@ export class AgentStateManager { this.subagentCompletedThresholdMs = opts.subagentCompletedThresholdMs ?? 5 * 60_000; this.subagentBusyCompletedThresholdMs = opts.subagentBusyCompletedThresholdMs ?? 15 * 60_000; this.livenessOracle = opts.livenessOracle; + this.displayNameOracle = opts.displayNameOracle; } setLivenessOracle(oracle: SessionLivenessOracle | undefined): void { this.livenessOracle = oracle; } + setDisplayNameOracle(oracle: SessionDisplayNameOracle | undefined): void { + this.displayNameOracle = oracle; + } + processEvent( event: ParsedEvent, configDir = '', @@ -129,6 +147,7 @@ export class AgentStateManager { this.applyTurnAndError(agent, event); // Liveness wins over turn-end: a dead pid can't be mid-turn. this.applyLivenessOverride(agent); + this.applyDisplayName(agent); return { agent, isNew: true }; } @@ -141,6 +160,7 @@ export class AgentStateManager { this.applyTurnAndError(existing, event); this.applyLivenessOverride(existing); } + this.applyDisplayName(existing); return { agent: existing, isNew: false }; } @@ -150,14 +170,35 @@ export class AgentStateManager { for (const agent of this.agents.values()) { const before = agent.status; const beforeActivity = agent.currentActivity; + const beforeName = agent.name; this.applyDerivedStatus(agent); - if (agent.status !== before || agent.currentActivity !== beforeActivity) { + this.applyDisplayName(agent); + if ( + agent.status !== before || + agent.currentActivity !== beforeActivity || + agent.name !== beforeName + ) { changed.push(agent.id); } } return changed; } + /** + * Reconcile `agent.name` against the oracle on every tick. Returning to the + * derived (slug/cwd) name is just as important as overlaying the oracle's + * label — without the fallback path, a removed/corrupt `state.json` would + * leave the previous title frozen on the sprite. Subagents are skipped + * (they have no jobId of their own and keep the filename descriptor). + */ + private applyDisplayName(agent: AgentState): void { + if (isSubagentSessionId(agent.id)) return; + const oracleName = this.displayNameOracle?.getDisplayName(agent.id); + agent.name = oracleName !== undefined && oracleName.length > 0 + ? oracleName + : agent.derivedName; + } + /** * Liveness override: when the oracle has at least one live session tracked, * any regular (non-subagent) sessionId that doesn't match a live Claude pid @@ -419,14 +460,15 @@ export class AgentStateManager { source: AgentSource, nameOverride?: string, ): AgentState { - const name = nameOverride !== undefined && nameOverride.length > 0 + const derivedName = nameOverride !== undefined && nameOverride.length > 0 ? nameOverride : deriveAgentName(event.slug, event.cwd, event.sessionId); const agent: AgentState = { id: event.sessionId, - name, + name: derivedName, + derivedName, heroClass: this.nextHeroClass(), - heroColor: this.pickUnusedColorFor(name), + heroColor: this.pickUnusedColorFor(derivedName), status: 'active', currentActivity: event.activity, currentFile: event.file, @@ -463,12 +505,14 @@ export class AgentStateManager { } // Subagents keep their filename-derived name — the event's slug is the - // parent session's slug (copied verbatim) and would be misleading. + // parent session's slug (copied verbatim) and would be misleading. Updates + // land on `derivedName` so `applyDisplayName()` can swap back when the + // oracle drops the user-set title. if (!isSubagentId(agent.id)) { if (event.slug !== undefined) { - agent.name = event.slug; - } else if (looksLikeSessionPrefix(agent.name) && event.cwd !== undefined) { - agent.name = cwdBasename(event.cwd); + agent.derivedName = event.slug; + } else if (looksLikeSessionPrefix(agent.derivedName) && event.cwd !== undefined) { + agent.derivedName = cwdBasename(event.cwd); } } diff --git a/server/src/types.ts b/server/src/types.ts index 32704fd..b862ee6 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -38,7 +38,20 @@ export interface ToolCall { // --- Core agent state --- export interface AgentState { id: string; // sessionId - name: string; // slug from JSONL (e.g. "bubbly-waddling-cat") + /** + * Label rendered above the hero. Starts as the JSONL slug or cwd basename + * (see `derivedName`), and is overlaid with the user-set display title + * whenever a `SessionDisplayNameOracle` returns one. Falls back to + * `derivedName` the moment the oracle drops the title — otherwise a deleted + * `state.json` would freeze a stale label on the sprite forever. + */ + name: string; + /** + * Slug/cwd-derived label that survives oracle gaps. Recomputed on every + * non-subagent JSONL event so a renamed cwd or a slug surfacing late still + * lands here without waiting for the oracle to come back. + */ + derivedName: string; heroClass: HeroClass; heroColor: HeroColor; status: 'active' | 'waiting' | 'idle' | 'completed' | 'error'; From 5fefc47aee1e6f2cdc7ed873509a61d7e450956f Mon Sep 17 00:00:00 2001 From: "JeongMinseok(Nalo)" Date: Tue, 19 May 2026 18:05:49 +0900 Subject: [PATCH 03/16] feat: visually distinguish sub-agent sessions as companion sprites (#3) (#6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server propagates parent ↔ child session relationship as domain data so the client no longer infers sub-agent status from sessionId patterns. Sub-agent heroes render at 0.7× scale and follow their parent hero with a deterministic fan-out offset; orphaned sub-agents fall back to regular slot-based layout. Server - AgentState.isSubagent + parentSessionId carry sub-agent metadata. - file-watcher.ts treats `/subagents/` as an anti-corruption boundary: disk layout becomes a SubagentContext domain value handed up the provider chain. - ClaudeProvider, ProviderHandlers, AgentStateManager.processEvent and index.ts forward SubagentContext end to end. Client - AgentState mirror gains the two fields; normalizeAgentState() shim tolerates older payloads. - HeroSprite uses computeSpriteScale(base, isSubagent); setHeroScale() preserves the sub-agent factor on runtime resize. - VillageScene tracks parentChildren / subagentParents separately from buildingSlots. Attached sub-agents bypass slot layout and follow the parent each frame via computeChildOffset() + teleportTo(). Tests - agent-state-manager.test.ts covers main / subagent / orphan / update paths. - hero-scale.test.ts pins the SUBAGENT_SCALE_FACTOR contract. - subagent-layout.test.ts pins fan-out determinism and stable childIndexOf() ordering. Closes #3 --- client/src/game/entities/HeroSprite.ts | 41 ++++- client/src/game/entities/hero-scale.test.ts | 28 ++++ client/src/game/entities/hero-scale.ts | 20 +++ client/src/game/scenes/VillageScene.ts | 156 ++++++++++++++++-- .../src/game/scenes/subagent-layout.test.ts | 74 +++++++++ client/src/game/scenes/subagent-layout.ts | 58 +++++++ client/src/hooks/useAgentState.ts | 11 +- client/src/types/agent.ts | 25 +++ server/src/index.ts | 16 +- server/src/providers/claude-provider.ts | 8 +- server/src/providers/types.ts | 15 +- server/src/state/agent-state-manager.test.ts | 76 +++++++++ server/src/state/agent-state-manager.ts | 8 +- server/src/types.ts | 26 +++ server/src/watchers/file-watcher.ts | 50 +++++- 15 files changed, 570 insertions(+), 42 deletions(-) create mode 100644 client/src/game/entities/hero-scale.test.ts create mode 100644 client/src/game/entities/hero-scale.ts create mode 100644 client/src/game/scenes/subagent-layout.test.ts create mode 100644 client/src/game/scenes/subagent-layout.ts diff --git a/client/src/game/entities/HeroSprite.ts b/client/src/game/entities/HeroSprite.ts index 7d0d0bc..e3635c2 100644 --- a/client/src/game/entities/HeroSprite.ts +++ b/client/src/game/entities/HeroSprite.ts @@ -4,6 +4,7 @@ import { getActiveTheme } from '../themes/registry'; import { findRoadPath, type Point } from '../data/road-network'; import { addCrispText } from '../text'; import { truncateLabel } from '../../utils/truncateLabel'; +import { computeSpriteScale } from './hero-scale'; const MOVE_SPEED = 150; /** Ground distance covered by one full run-cycle. Keeps legs synced to travel. */ @@ -136,9 +137,11 @@ export class HeroSprite { this.runKey = cfg.runKey; this.facesLeft = cfg.facesLeft; - // Create sprite with idle animation + // Create sprite with idle animation. Sub-agents render at a + // smaller scale so the eye reads them as companions/helpers of the + // main hero — see `computeSpriteScale` for the rule. this.sprite = scene.add.sprite(x, y, this.idleKey); - this.sprite.setScale(theme.heroScale); + this.sprite.setScale(computeSpriteScale(theme.heroScale, isSubagent)); // Flip sprites that natively face left so they face right by default this.sprite.setFlipX(this.facesLeft); if (cfg.tint !== null) this.sprite.setTint(cfg.tint); @@ -249,9 +252,39 @@ export class HeroSprite { get x(): number { return this._x; } get y(): number { return this._y; } - /** Override the default hero scale (e.g. from MapConfig settings). */ + /** + * Override the default hero scale (e.g. from MapConfig settings). + * Sub-agent factor is preserved — passing `1.5` on a sub-agent yields an + * effective sprite scale of `1.5 * SUBAGENT_SCALE_FACTOR`. + */ setHeroScale(scale: number): void { - this.sprite.setScale(scale); + this.sprite.setScale(computeSpriteScale(scale, this.isSubagent)); + } + + /** + * Teleport the hero (sprite + every overlay text) to a new position. Used + * by the scene to keep attached sub-agents stuck to their parent — these + * sprites bypass the road-network tween path entirely and follow parent + * position frame to frame. + */ + teleportTo(x: number, y: number): void { + this._x = x; + this._y = y; + this.sprite.setPosition(x, y); + this.nameText.setPosition(x, y + this.nameOffsetY); + this.layoutSubagentAndSource(); + this.layoutActivityAndModel(); + this.detailText.setPosition(x, y + this.detailOffsetY); + this.taskText.setPosition(x, y + this.taskOffsetY); + if (this.selectionHalo !== null) { + this.selectionHalo.setPosition(x, y); + } + this.updateDepth(); + } + + /** Whether this hero represents a sub-agent (visual companion). */ + getIsSubagent(): boolean { + return this.isSubagent; } /** Make the sprite respond to pointerdown with the supplied callback. */ diff --git a/client/src/game/entities/hero-scale.test.ts b/client/src/game/entities/hero-scale.test.ts new file mode 100644 index 0000000..3b5e436 --- /dev/null +++ b/client/src/game/entities/hero-scale.test.ts @@ -0,0 +1,28 @@ +import { describe, test, expect } from 'bun:test'; +import { computeSpriteScale, SUBAGENT_SCALE_FACTOR } from './hero-scale'; + +describe('computeSpriteScale', () => { + test('main session keeps base scale', () => { + expect(computeSpriteScale(1.0, false)).toBe(1.0); + expect(computeSpriteScale(2.0, false)).toBe(2.0); + }); + + test('sub-agent shrinks to SUBAGENT_SCALE_FACTOR × base', () => { + expect(computeSpriteScale(1.0, true)).toBeCloseTo(SUBAGENT_SCALE_FACTOR); + expect(computeSpriteScale(2.0, true)).toBeCloseTo(2.0 * SUBAGENT_SCALE_FACTOR); + }); + + test('SUBAGENT_SCALE_FACTOR is between 0 and 1 (companion, not invisible)', () => { + expect(SUBAGENT_SCALE_FACTOR).toBeGreaterThan(0); + expect(SUBAGENT_SCALE_FACTOR).toBeLessThan(1); + }); + + test('scale composes — runtime resize via setHeroScale preserves factor', () => { + const isSubagent = true; + const baseScales = [0.5, 1.0, 1.5, 3.0]; + for (const base of baseScales) { + const effective = computeSpriteScale(base, isSubagent); + expect(effective).toBeCloseTo(base * SUBAGENT_SCALE_FACTOR); + } + }); +}); diff --git a/client/src/game/entities/hero-scale.ts b/client/src/game/entities/hero-scale.ts new file mode 100644 index 0000000..d36fc5e --- /dev/null +++ b/client/src/game/entities/hero-scale.ts @@ -0,0 +1,20 @@ +/** + * Visual scale rules for hero sprites. Pure functions — no Phaser dependency + * so unit tests can pin the visual contract without spinning up a scene. + */ + +/** + * Multiplicative scale applied to a sub-agent sprite relative to a regular + * hero. 0.7 is small enough to read as "companion / helper" without making + * the sprite illegible. + */ +export const SUBAGENT_SCALE_FACTOR = 0.7; + +/** + * Effective sprite scale for a hero, given the theme's base hero scale and + * whether the hero is a sub-agent. Main session: returns `baseScale`. + * Sub-agent: returns `baseScale * SUBAGENT_SCALE_FACTOR`. + */ +export function computeSpriteScale(baseScale: number, isSubagent: boolean): number { + return isSubagent ? baseScale * SUBAGENT_SCALE_FACTOR : baseScale; +} diff --git a/client/src/game/scenes/VillageScene.ts b/client/src/game/scenes/VillageScene.ts index 52e8dc7..352a288 100644 --- a/client/src/game/scenes/VillageScene.ts +++ b/client/src/game/scenes/VillageScene.ts @@ -13,6 +13,8 @@ import type { AssetManifest, MapConfig, BuildingPosition, NpcPlacement } from '. import { SERVER_URL as API_BASE } from '../../config'; import { getActiveTheme, rebaseSavedScale } from '../themes/registry'; import { computeShowSourceBadge } from '../../presentation/agentPresentation'; +import { TILE_SIZE } from '../../editor/types/map'; +import { computeChildOffset } from './subagent-layout'; /** Set `cam.zoom` to `newZoom` while keeping the world point currently * under screen coordinates (sx, sy) pinned to the same screen spot. @@ -47,6 +49,17 @@ export class VillageScene extends Phaser.Scene { /** Tracks which building each hero is currently assigned to. */ private heroBuildingMap = new Map(); + /** + * Parent → set of attached sub-agent ids. Attached sub-agents bypass + * `buildingSlots` entirely and follow their parent's position frame to + * frame. Orphan sub-agents (parent gone or never seen) fall back to + * regular slot assignment and are absent from this map. + */ + private parentChildren = new Map>(); + + /** Reverse lookup: attached child id → parent id. */ + private subagentParents = new Map(); + /** Atmospheric effects */ private nightOverlay: Phaser.GameObjects.Graphics | null = null; private rainEmitter: Phaser.GameObjects.Particles.ParticleEmitter | null = null; @@ -381,6 +394,8 @@ export class VillageScene extends Phaser.Scene { this.buildings = []; this.buildingSlots.clear(); this.heroBuildingMap.clear(); + this.parentChildren.clear(); + this.subagentParents.clear(); for (const npc of this.editorNpcs) npc.destroy(); this.editorNpcs = []; }; @@ -572,6 +587,65 @@ export class VillageScene extends Phaser.Scene { // Slot management // --------------------------------------------------------------------------- + /** + * Is this agent eligible to render as an attached sub-agent (companion of + * a live parent)? Returns true only when a parent hero is currently on + * screen — orphan sub-agents fall through to regular slot layout. + */ + private canAttachSubagent(agent: AgentState): boolean { + if (!agent.isSubagent) return false; + if (agent.parentSessionId === undefined) return false; + return this.heroes.has(agent.parentSessionId); + } + + private attachSubagent(parentId: string, childId: string): void { + this.subagentParents.set(childId, parentId); + let siblings = this.parentChildren.get(parentId); + if (siblings === undefined) { + siblings = new Set(); + this.parentChildren.set(parentId, siblings); + } + siblings.add(childId); + } + + private detachSubagent(childId: string): void { + const parentId = this.subagentParents.get(childId); + if (parentId === undefined) return; + this.subagentParents.delete(childId); + const siblings = this.parentChildren.get(parentId); + if (siblings === undefined) return; + siblings.delete(childId); + if (siblings.size === 0) { + this.parentChildren.delete(parentId); + } + } + + /** + * Each frame, anchor every attached sub-agent to its parent's current + * position plus a deterministic stagger offset. Sub-agents bypass the + * tween-driven road network, so this is what actually makes them follow. + */ + override update(): void { + if (this.parentChildren.size === 0) return; + for (const [parentId, siblings] of this.parentChildren) { + const parentHero = this.heroes.get(parentId); + if (parentHero === undefined) continue; + const sortedSiblings = [...siblings].sort(); + for (let i = 0; i < sortedSiblings.length; i++) { + const childId = sortedSiblings[i]; + if (childId === undefined) continue; + const child = this.heroes.get(childId); + if (child === undefined) continue; + const offset = computeChildOffset( + { x: parentHero.x, y: parentHero.y }, + i, + TILE_SIZE, + ); + child.teleportTo(offset.x, offset.y); + } + } + } + private addToSlot(buildingId: string, heroId: string): void { let slots = this.buildingSlots.get(buildingId); if (slots === undefined) { @@ -652,9 +726,35 @@ export class VillageScene extends Phaser.Scene { // against the unfiltered `agents` snapshot. const showSourceBadge = computeShowSourceBadge(agents); + // Track which buildings need repositioning. Declared up here because both + // the removal pass (parent leaves → children orphan into slots) and the + // visible-iteration pass below need to flag dirty buildings. + const buildingsToReposition = new Set(); + // Remove heroes no longer visible for (const [id, hero] of this.heroes) { if (!visible.some((a) => a.id === id)) { + // If a parent goes away, promote each attached child to orphan so it + // re-enters slot-based layout instead of vanishing along with the parent. + const orphanedChildren = this.parentChildren.get(id); + if (orphanedChildren !== undefined) { + for (const childId of orphanedChildren) { + this.subagentParents.delete(childId); + const childHero = this.heroes.get(childId); + const childAgent = visible.find((a) => a.id === childId); + if (childHero !== undefined && childAgent !== undefined) { + const childBuildingDef = getBuildingForActivity(childAgent.currentActivity); + this.addToSlot(childBuildingDef.id, childId); + buildingsToReposition.add(childBuildingDef.id); + } + } + this.parentChildren.delete(id); + } + + // If the leaving hero was itself an attached sub-agent, detach cleanly + // and re-layout the remaining siblings so the deterministic order tightens. + this.detachSubagent(id); + const oldBuilding = this.removeFromSlot(id); hero.destroy(); this.heroes.delete(id); @@ -664,15 +764,14 @@ export class VillageScene extends Phaser.Scene { } } - // Track which buildings need repositioning - const buildingsToReposition = new Set(); - for (const agent of visible) { const existing = this.heroes.get(agent.id); const buildingDef = getBuildingForActivity(agent.currentActivity); + const attachable = this.canAttachSubagent(agent); if (existing === undefined) { - // New hero: spawn at configured spawn point then assign to building + // New hero: spawn at configured spawn point then either follow the + // parent (attached sub-agent) or take a regular building slot. const hero = new HeroSprite( this, agent.id, @@ -681,7 +780,7 @@ export class VillageScene extends Phaser.Scene { agent.heroColor, this.heroSpawn.x, this.heroSpawn.y, - agent.id.startsWith('agent-'), + agent.isSubagent, agent.source, ); hero.setHeroScale(this.heroScale); @@ -695,8 +794,13 @@ export class VillageScene extends Phaser.Scene { hero.setInteractiveForSelection(() => { eventBridge.emit('hero:clicked', agent.id); }); - this.addToSlot(buildingDef.id, agent.id); - buildingsToReposition.add(buildingDef.id); + + if (attachable) { + this.attachSubagent(agent.parentSessionId as string, agent.id); + } else { + this.addToSlot(buildingDef.id, agent.id); + buildingsToReposition.add(buildingDef.id); + } } else { // Always update detail text (file/command changes even without activity change) existing.updateName(agent.name); @@ -706,20 +810,38 @@ export class VillageScene extends Phaser.Scene { existing.setErrorTimestamp(agent.lastErrorAt); existing.setModel(agent.model); - const currentBuildingId = this.heroBuildingMap.get(agent.id); - - if (currentBuildingId !== buildingDef.id) { - // Hero changed building — update activity before repositioning + const wasAttached = this.subagentParents.has(agent.id); + if (wasAttached && !attachable) { + // Orphaned mid-life — leave the parent group and fall back to slot. + this.detachSubagent(agent.id); existing.setActivity(agent.currentActivity); - const oldBuilding = this.removeFromSlot(agent.id); this.addToSlot(buildingDef.id, agent.id); - - if (oldBuilding !== undefined) { - buildingsToReposition.add(oldBuilding); - } buildingsToReposition.add(buildingDef.id); + } else if (!wasAttached && attachable && this.heroBuildingMap.has(agent.id)) { + // Newly attachable (parent appeared) — vacate slot and follow parent. + const oldBuilding = this.removeFromSlot(agent.id); + if (oldBuilding !== undefined) buildingsToReposition.add(oldBuilding); + this.attachSubagent(agent.parentSessionId as string, agent.id); + } else if (!wasAttached) { + const currentBuildingId = this.heroBuildingMap.get(agent.id); + + if (currentBuildingId !== buildingDef.id) { + // Hero changed building — update activity before repositioning + existing.setActivity(agent.currentActivity); + const oldBuilding = this.removeFromSlot(agent.id); + this.addToSlot(buildingDef.id, agent.id); + + if (oldBuilding !== undefined) { + buildingsToReposition.add(oldBuilding); + } + buildingsToReposition.add(buildingDef.id); + } else if (existing.currentActivity !== agent.currentActivity) { + // Same building, different activity — update label + existing.setActivity(agent.currentActivity); + } } else if (existing.currentActivity !== agent.currentActivity) { - // Same building, different activity — update label + // Attached sub-agent — keep activity label up to date; layout is + // driven by the parent's position each frame. existing.setActivity(agent.currentActivity); } } diff --git a/client/src/game/scenes/subagent-layout.test.ts b/client/src/game/scenes/subagent-layout.test.ts new file mode 100644 index 0000000..1e8a53d --- /dev/null +++ b/client/src/game/scenes/subagent-layout.test.ts @@ -0,0 +1,74 @@ +import { describe, test, expect } from 'bun:test'; +import { + CHILD_STAGGER_TILE_OFFSET, + FIRST_CHILD_TILE_OFFSET, + childIndexOf, + computeChildOffset, +} from './subagent-layout'; + +describe('computeChildOffset', () => { + test('first child sits one offset to the parent right, same Y', () => { + const result = computeChildOffset({ x: 100, y: 200 }, 0, 32); + expect(result.x).toBeCloseTo(100 + FIRST_CHILD_TILE_OFFSET * 32); + expect(result.y).toBe(200); + }); + + test('second child stays staggered by one tile beyond the first', () => { + const result = computeChildOffset({ x: 100, y: 200 }, 1, 32); + const expectedGap = + FIRST_CHILD_TILE_OFFSET + CHILD_STAGGER_TILE_OFFSET; + expect(result.x).toBeCloseTo(100 + expectedGap * 32); + expect(result.y).toBe(200); + }); + + test('third child continues deterministic fan-out', () => { + const result = computeChildOffset({ x: 100, y: 200 }, 2, 32); + const expectedGap = + FIRST_CHILD_TILE_OFFSET + 2 * CHILD_STAGGER_TILE_OFFSET; + expect(result.x).toBeCloseTo(100 + expectedGap * 32); + }); + + test('deterministic — same inputs yield same outputs', () => { + const a = computeChildOffset({ x: 50, y: 75 }, 3, 16); + const b = computeChildOffset({ x: 50, y: 75 }, 3, 16); + expect(a).toEqual(b); + }); + + test('scales with tile width', () => { + const small = computeChildOffset({ x: 0, y: 0 }, 0, 10); + const large = computeChildOffset({ x: 0, y: 0 }, 0, 100); + expect(large.x).toBeCloseTo(small.x * 10); + }); +}); + +describe('childIndexOf', () => { + test('returns lexicographically sorted index', () => { + const siblings = ['agent-c', 'agent-a', 'agent-b']; + expect(childIndexOf('agent-a', siblings)).toBe(0); + expect(childIndexOf('agent-b', siblings)).toBe(1); + expect(childIndexOf('agent-c', siblings)).toBe(2); + }); + + test('stable across input order — adding a sibling keeps existing slots', () => { + const before = ['agent-b', 'agent-d']; + const after = ['agent-d', 'agent-b', 'agent-a']; + expect(childIndexOf('agent-b', before)).toBe(0); + expect(childIndexOf('agent-d', before)).toBe(1); + // After 'agent-a' joins, 'agent-b' shifts to index 1, 'agent-d' to 2. + // 'agent-a' takes the now-leading slot. + expect(childIndexOf('agent-a', after)).toBe(0); + expect(childIndexOf('agent-b', after)).toBe(1); + expect(childIndexOf('agent-d', after)).toBe(2); + }); + + test('returns -1 when sessionId is not in the sibling list', () => { + expect(childIndexOf('agent-z', ['agent-a', 'agent-b'])).toBe(-1); + }); + + test('input array is not mutated by sorting', () => { + const original = ['agent-c', 'agent-a', 'agent-b']; + const snapshot = [...original]; + childIndexOf('agent-a', original); + expect(original).toEqual(snapshot); + }); +}); diff --git a/client/src/game/scenes/subagent-layout.ts b/client/src/game/scenes/subagent-layout.ts new file mode 100644 index 0000000..fbca3a2 --- /dev/null +++ b/client/src/game/scenes/subagent-layout.ts @@ -0,0 +1,58 @@ +/** + * Pure helpers for placing sub-agent sprites relative to their parent hero. + * + * Kept Phaser-free so unit tests can pin the visual contract without a scene. + * VillageScene calls these to decide where each attached sub-agent should + * render frame to frame. + */ + +/** Horizontal offset (in tile widths) for the *first* attached sub-agent. */ +export const FIRST_CHILD_TILE_OFFSET = 0.7; + +/** Incremental tile-width gap between successive sub-agents of the same parent. */ +export const CHILD_STAGGER_TILE_OFFSET = 0.7; + +export interface Point { + x: number; + y: number; +} + +/** + * Compute the screen position for an attached sub-agent given the parent + * hero's current position, the sub-agent's index among siblings (0-based, + * stable sort by sessionId), and the tile width in pixels. + * + * Layout: sub-agents fan out to the parent's right at evenly spaced + * intervals. Index 0 sits one offset to the right, index 1 two offsets, + * etc. Y matches the parent so feet stay on the same ground line. + * + * Deterministic — same inputs always yield the same position, so update + * cycles don't jitter the layout. + */ +export function computeChildOffset( + parentPos: Point, + childIndex: number, + tileWidth: number, +): Point { + const horizontalGap = + FIRST_CHILD_TILE_OFFSET + childIndex * CHILD_STAGGER_TILE_OFFSET; + return { + x: parentPos.x + horizontalGap * tileWidth, + y: parentPos.y, + }; +} + +/** + * Determine the stable child index of `childSessionId` among a parent's + * sub-agents. Siblings are sorted lexicographically by session id so that + * adding or removing a sibling re-uses existing slots predictably. + * + * Returns -1 if `childSessionId` is not present in `siblingSessionIds`. + */ +export function childIndexOf( + childSessionId: string, + siblingSessionIds: readonly string[], +): number { + const sorted = [...siblingSessionIds].sort(); + return sorted.indexOf(childSessionId); +} diff --git a/client/src/hooks/useAgentState.ts b/client/src/hooks/useAgentState.ts index 992d8e9..e263545 100644 --- a/client/src/hooks/useAgentState.ts +++ b/client/src/hooks/useAgentState.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useRef, useCallback } from 'react'; import type { AgentState, WsEvent, ActivityLogEntry } from '../types/agent'; +import { normalizeAgentState } from '../types/agent'; import { eventBridge } from '../game/EventBridge'; import { WS_URL } from '../config'; @@ -27,19 +28,21 @@ export function useAgentState(): AgentStateHook { const handleEvent = useCallback((event: WsEvent) => { switch (event.type) { case 'snapshot': - setAgents(event.agents); + setAgents(event.agents.map(normalizeAgentState)); setConfigDirs(event.configDirs); break; case 'agent:new': - setAgents((prev) => [...prev, event.agent]); + setAgents((prev) => [...prev, normalizeAgentState(event.agent)]); break; - case 'agent:update': + case 'agent:update': { + const normalized = normalizeAgentState(event.agent); setAgents((prev) => - prev.map((a) => (a.id === event.agent.id ? event.agent : a)), + prev.map((a) => (a.id === normalized.id ? normalized : a)), ); break; + } case 'agent:complete': setAgents((prev) => diff --git a/client/src/types/agent.ts b/client/src/types/agent.ts index d524ec1..0d39c83 100644 --- a/client/src/types/agent.ts +++ b/client/src/types/agent.ts @@ -101,6 +101,31 @@ export interface AgentState { source: AgentSource; /** Raw model id from Claude Code (e.g. `claude-opus-4-6`). Undefined for Codex. */ model?: string; + /** + * True when this session was spawned by another session as a sub-agent. + * Server-computed (file-system layout). Client never inspects sessionId + * patterns directly. Older server payloads predating this field are + * tolerated by `normalizeAgentState` below. + */ + isSubagent: boolean; + /** + * Parent session id when this session is a sub-agent and the parent is + * known. Undefined for main sessions and for orphan sub-agents. + */ + parentSessionId?: string; +} + +/** + * Normalize a partial AgentState received over WebSocket — back-compat shim + * for older server payloads (or replays) that do not carry the new sub-agent + * fields. Anything missing falls back to "main session" semantics so the + * existing rendering path stays correct. + */ +export function normalizeAgentState(raw: Omit & { isSubagent?: boolean }): AgentState { + return { + ...raw, + isSubagent: raw.isSubagent ?? false, + }; } /** diff --git a/server/src/index.ts b/server/src/index.ts index a531f59..304710f 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -120,7 +120,13 @@ const providerHandlers: ProviderHandlers = { // filename-derived id so each subagent becomes its own hero instead // of being folded into the parent agent. event.sessionId = payload.sessionId; - const result = stateManager.processEvent(event, payload.configDir, payload.source, payload.nameOverride); + const result = stateManager.processEvent( + event, + payload.configDir, + payload.source, + payload.nameOverride, + payload.subagentCtx, + ); if (result !== null && result.isNew) { wsServer.broadcastNewAgent(result.agent); } @@ -134,7 +140,13 @@ const providerHandlers: ProviderHandlers = { for (const event of payload.events) { // See note in onSessionStart: rekey to filename-derived id for subagents. event.sessionId = payload.sessionId; - const result = stateManager.processEvent(event, payload.configDir, payload.source); + const result = stateManager.processEvent( + event, + payload.configDir, + payload.source, + undefined, + payload.subagentCtx, + ); if (result === null) continue; // resume-hint dump, no state change if (result.isNew) { diff --git a/server/src/providers/claude-provider.ts b/server/src/providers/claude-provider.ts index 2d8fa59..937a273 100644 --- a/server/src/providers/claude-provider.ts +++ b/server/src/providers/claude-provider.ts @@ -26,10 +26,10 @@ export class ClaudeProvider implements SessionProvider { maxAgeMs: this.opts.maxAgeMs, claudeDirs: this.opts.claudeDirs, - onNewSession: async (sessionId, filePath, configDir) => { + onNewSession: async (sessionId, filePath, configDir, subagentCtx) => { const contents = await Bun.file(filePath).text(); const events = parseSessionFile(contents); - const nameOverride = sessionId.startsWith('agent-') + const nameOverride = subagentCtx.isSubagent ? await resolveSubagentLabel(filePath).catch(() => undefined) : undefined; await handlers.onSessionStart({ @@ -38,10 +38,11 @@ export class ClaudeProvider implements SessionProvider { configDir, events, nameOverride, + subagentCtx, }); }, - onSessionUpdate: (sessionId, _filePath, newContent, configDir) => { + onSessionUpdate: (sessionId, _filePath, newContent, configDir, subagentCtx) => { const events: ParsedEvent[] = []; for (const line of newContent.split('\n')) { if (line.trim() === '') continue; @@ -54,6 +55,7 @@ export class ClaudeProvider implements SessionProvider { sessionId, configDir, events, + subagentCtx, }); }, }); diff --git a/server/src/providers/types.ts b/server/src/providers/types.ts index 979711f..3cf43f3 100644 --- a/server/src/providers/types.ts +++ b/server/src/providers/types.ts @@ -1,4 +1,4 @@ -import type { AgentSource } from '../types'; +import type { AgentSource, SubagentContext } from '../types'; import type { ParsedEvent } from '../parsers/session-parser'; export interface SessionStartPayload { @@ -8,6 +8,13 @@ export interface SessionStartPayload { events: ParsedEvent[]; /** Optional name override resolved by the provider (e.g. Claude subagent label). */ nameOverride?: string; + /** + * Subagent context resolved by the provider — `isSubagent` is always set; + * `parentSessionId` is set only when discoverable from disk layout. + * Defaults to `{ isSubagent: false }` for main sessions and for providers + * that have no sub-agent concept (e.g. Codex). + */ + subagentCtx?: SubagentContext; } export interface SessionEventsPayload { @@ -16,6 +23,12 @@ export interface SessionEventsPayload { configDir: string; /** Incremental events since the last update. */ events: ParsedEvent[]; + /** + * Subagent context — propagated on updates so that any first-event-on-update + * path (extremely rare) sees the same metadata. Existing agents already have + * the fields set from `onSessionStart`. + */ + subagentCtx?: SubagentContext; } export interface ProviderHandlers { diff --git a/server/src/state/agent-state-manager.test.ts b/server/src/state/agent-state-manager.test.ts index 8005cd4..a531c86 100644 --- a/server/src/state/agent-state-manager.test.ts +++ b/server/src/state/agent-state-manager.test.ts @@ -857,4 +857,80 @@ describe('AgentStateManager', () => { expect(changed).toContain('sess-1'); expect(m.getAgent('sess-1')!.name).toBe('[#1, 5/13] late wiring'); }); + + describe('subagent context propagation', () => { + test('main session has isSubagent=false and parentSessionId undefined when no ctx passed', () => { + const mgr = new AgentStateManager(); + const result = mgr.processEvent(makeEvent({ sessionId: 'main-1' })); + + expect(result).not.toBeNull(); + expect(result!.agent.isSubagent).toBe(false); + expect(result!.agent.parentSessionId).toBeUndefined(); + }); + + test('subagent registered with ctx records both isSubagent and parentSessionId', () => { + const mgr = new AgentStateManager(); + const result = mgr.processEvent( + makeEvent({ sessionId: 'agent-aXYZ12345678abcd' }), + '/home/x/.claude', + 'claude', + undefined, + { isSubagent: true, parentSessionId: 'parent-sess-1' }, + ); + + expect(result).not.toBeNull(); + expect(result!.agent.isSubagent).toBe(true); + expect(result!.agent.parentSessionId).toBe('parent-sess-1'); + }); + + test('orphan subagent (ctx has isSubagent but no parentSessionId) keeps isSubagent=true', () => { + const mgr = new AgentStateManager(); + const result = mgr.processEvent( + makeEvent({ sessionId: 'agent-bORPHAN1234abcdef' }), + '/home/x/.claude', + 'claude', + undefined, + { isSubagent: true }, + ); + + expect(result).not.toBeNull(); + expect(result!.agent.isSubagent).toBe(true); + expect(result!.agent.parentSessionId).toBeUndefined(); + }); + + test('main session with explicit ctx { isSubagent: false } sets fields correctly', () => { + const mgr = new AgentStateManager(); + const result = mgr.processEvent( + makeEvent({ sessionId: 'main-2' }), + '/home/x/.claude', + 'claude', + undefined, + { isSubagent: false }, + ); + + expect(result).not.toBeNull(); + expect(result!.agent.isSubagent).toBe(false); + expect(result!.agent.parentSessionId).toBeUndefined(); + }); + + test('updates to existing subagent preserve isSubagent and parentSessionId', () => { + const mgr = new AgentStateManager(); + mgr.processEvent( + makeEvent({ sessionId: 'agent-cUPDATE12345678ab' }), + '/home/x/.claude', + 'claude', + undefined, + { isSubagent: true, parentSessionId: 'parent-2' }, + ); + const result = mgr.processEvent( + makeEvent({ sessionId: 'agent-cUPDATE12345678ab', activity: 'editing', file: '/bar.ts' }), + ); + + expect(result).not.toBeNull(); + expect(result!.isNew).toBe(false); + expect(result!.agent.isSubagent).toBe(true); + expect(result!.agent.parentSessionId).toBe('parent-2'); + expect(result!.agent.currentActivity).toBe('editing'); + }); + }); }); diff --git a/server/src/state/agent-state-manager.ts b/server/src/state/agent-state-manager.ts index 3137ced..f741fd0 100644 --- a/server/src/state/agent-state-manager.ts +++ b/server/src/state/agent-state-manager.ts @@ -1,4 +1,4 @@ -import type { AgentSource, AgentState, HeroClass, HeroColor } from '../types'; +import type { AgentSource, AgentState, HeroClass, HeroColor, SubagentContext } from '../types'; import { HERO_CLASSES, HERO_COLORS } from '../types'; import type { ParsedEvent } from '../parsers/session-parser'; @@ -127,6 +127,7 @@ export class AgentStateManager { configDir = '', source: AgentSource = 'claude', nameOverride?: string, + subagentCtx?: SubagentContext, ): ProcessResult | null { const existing = this.agents.get(event.sessionId); @@ -141,7 +142,7 @@ export class AgentStateManager { } if (existing === undefined) { - const agent = this.createAgent(event, configDir, source, nameOverride); + const agent = this.createAgent(event, configDir, source, nameOverride, subagentCtx); this.agents.set(event.sessionId, agent); this.applyDerivedStatus(agent); this.applyTurnAndError(agent, event); @@ -459,6 +460,7 @@ export class AgentStateManager { configDir: string, source: AgentSource, nameOverride?: string, + subagentCtx?: SubagentContext, ): AgentState { const derivedName = nameOverride !== undefined && nameOverride.length > 0 ? nameOverride @@ -486,6 +488,8 @@ export class AgentStateManager { configDir, source, model: event.model, + isSubagent: subagentCtx?.isSubagent ?? false, + parentSessionId: subagentCtx?.parentSessionId, }; return agent; } diff --git a/server/src/types.ts b/server/src/types.ts index b862ee6..42ea137 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -79,6 +79,32 @@ export interface AgentState { * sessions and for Claude sessions whose JSONL predates the field. */ model?: string; + /** + * True when this session was spawned by another session as a sub-agent + * (e.g. Claude Code `Agent` / Task tool). Server computes from file-system + * layout (subagents/ directory) — client never inspects sessionId patterns + * directly. Defaults to false for main sessions and for Codex sessions + * (Codex has no sub-agent concept). + */ + isSubagent: boolean; + /** + * Parent session id when this session is a sub-agent and the parent is + * known. Undefined for main sessions and for orphan sub-agents whose + * parent jsonl could not be located. + */ + parentSessionId?: string; +} + +// --- Subagent context (anti-corruption boundary) --- +/** + * Carries server-domain knowledge about whether a JSONL file represents a + * sub-agent session, plus the parent session id when discoverable. The + * file-watcher computes this from disk layout once; downstream layers + * (provider, state manager) never inspect file paths or sessionId patterns. + */ +export interface SubagentContext { + isSubagent: boolean; + parentSessionId?: string; } // --- Session metadata from ~/.claude/sessions/.json diff --git a/server/src/watchers/file-watcher.ts b/server/src/watchers/file-watcher.ts index a0e4a90..8aeecc0 100644 --- a/server/src/watchers/file-watcher.ts +++ b/server/src/watchers/file-watcher.ts @@ -2,9 +2,22 @@ import { readdir, stat } from 'node:fs/promises'; import { join } from 'node:path'; import { homedir } from 'node:os'; +import type { SubagentContext } from '../types'; + export interface WatcherCallbacks { - onNewSession: (sessionId: string, projectPath: string, configDir: string) => void; - onSessionUpdate: (sessionId: string, projectPath: string, newContent: string, configDir: string) => void; + onNewSession: ( + sessionId: string, + projectPath: string, + configDir: string, + subagentCtx: SubagentContext, + ) => void; + onSessionUpdate: ( + sessionId: string, + projectPath: string, + newContent: string, + configDir: string, + subagentCtx: SubagentContext, + ) => void; } export interface WatcherOptions extends WatcherCallbacks { @@ -115,12 +128,21 @@ export class FileWatcher { for (const file of files) { if (file.endsWith('.jsonl')) { - await this.processJsonlFile(join(projectPath, file), file.replace('.jsonl', ''), claudeDir); + await this.processJsonlFile( + join(projectPath, file), + file.replace('.jsonl', ''), + claudeDir, + { isSubagent: false }, + ); continue; } - // Per-session subdirectories may contain subagent JSONLs at /subagents/agent-*.jsonl - const subagentsDir = join(projectPath, file, 'subagents'); + // Per-session subdirectories may contain subagent JSONLs at + // /subagents/agent-*.jsonl. The directory name *is* + // the parent session id — this is the anti-corruption boundary where + // disk layout becomes domain metadata (`SubagentContext`). + const parentSessionId = file; + const subagentsDir = join(projectPath, parentSessionId, 'subagents'); const subStat = await stat(subagentsDir).catch(() => null); if (subStat === null || !subStat.isDirectory()) continue; @@ -129,13 +151,23 @@ export class FileWatcher { for (const subFile of subFiles) { if (!subFile.endsWith('.jsonl')) continue; - await this.processJsonlFile(join(subagentsDir, subFile), subFile.replace('.jsonl', ''), claudeDir); + await this.processJsonlFile( + join(subagentsDir, subFile), + subFile.replace('.jsonl', ''), + claudeDir, + { isSubagent: true, parentSessionId }, + ); } } } } - private async processJsonlFile(filePath: string, sessionId: string, claudeDir: string): Promise { + private async processJsonlFile( + filePath: string, + sessionId: string, + claudeDir: string, + subagentCtx: SubagentContext, + ): Promise { const fileStat = await stat(filePath).catch(() => null); if (fileStat === null) return; @@ -153,14 +185,14 @@ export class FileWatcher { } // New session file this.fileSizes.set(filePath, currentSize); - this.callbacks.onNewSession(sessionId, filePath, claudeDir); + this.callbacks.onNewSession(sessionId, filePath, claudeDir, subagentCtx); } else if (currentSize > previousSize) { // File grew — read only the new bytes const fd = Bun.file(filePath); const newBytes = fd.slice(previousSize, currentSize); const newContent = await newBytes.text(); this.fileSizes.set(filePath, currentSize); - this.callbacks.onSessionUpdate(sessionId, filePath, newContent, claudeDir); + this.callbacks.onSessionUpdate(sessionId, filePath, newContent, claudeDir, subagentCtx); } } } From 7338481158cde917ca5aa00bdad2beea5020c472 Mon Sep 17 00:00:00 2001 From: "JeongMinseok(Nalo)" Date: Tue, 19 May 2026 19:16:22 +0900 Subject: [PATCH 04/16] [issues-7] Bound AgentState history + surface WS diagnostics (#8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(server): bound AgentState history and surface WS diagnostics (#7) After a few weeks of Claude/Codex sessions piling up on disk, the initial snapshot the server sends on WebSocket connect grew past 2.5 MB. The browser would then drop the WS immediately on connect, BootScene never received ws:connected, and the React overlay (Top Bar, Party Bar, new completed-agents toggle) never rendered. The runaway was `toolCalls` and `filesModified` on each AgentState growing without bound — provider replays the entire JSONL history on startup, and each Codex toolCall can carry several KB of aggregated output. A small helper now keeps each agent to the most recent 50 entries on both the create and the update path. Snapshot now also logs its byte size on every send, and any broadcast frame that crosses 500 KB warns — so the next time the payload grows the operator sees it before the browser does. The WS onclose handler on both sides now records code/reason/wasClean too, because the original symptom hunt was blocked on not knowing whether the browser closed for 1009 (too big), 1006 (abnormal), or something else. * fix(client): guard WS reconnect against StrictMode unmount race (#7) Codex senior review found this is almost certainly the real driver of the "connect → error → disconnect" loop, not the snapshot payload itself. `onclose` was arming a reconnect timer on every close — including the intentional close from the effect's cleanup path. On a React StrictMode mount→unmount→mount cycle (or Fast Refresh), the orphaned hook instance's timer would still fire, build a new WebSocket, and emit bridge events on behalf of a hook that no longer existed, racing the freshly-mounted one. A `shouldReconnect` ref tracks lifecycle state. `onclose` skips the reconnect when cleanup flipped the flag, and the cleanup path now also nulls out `wsRef.current` and the timer ref so a stale socket cannot drive the next reconnect. Also tightens the `MAX_*` comment in agent-state-manager — transport concerns (byte budget, frame size warn) belong to WebSocketServer; the state manager's cap is the retention policy of the AgentState model itself, not a workaround for a particular transport. --- client/src/hooks/useAgentState.ts | 34 ++++++- server/src/index.ts | 4 +- server/src/state/agent-state-manager.test.ts | 98 ++++++++++++++++++++ server/src/state/agent-state-manager.ts | 20 ++++ server/src/ws/websocket-server.ts | 25 ++++- 5 files changed, 173 insertions(+), 8 deletions(-) diff --git a/client/src/hooks/useAgentState.ts b/client/src/hooks/useAgentState.ts index e263545..2eb5f59 100644 --- a/client/src/hooks/useAgentState.ts +++ b/client/src/hooks/useAgentState.ts @@ -24,6 +24,14 @@ export function useAgentState(): AgentStateHook { const [configDirs, setConfigDirs] = useState(null); const wsRef = useRef(null); const reconnectTimer = useRef | null>(null); + // Distinguishes "the effect cleanup intentionally closed the socket" from + // "the server or network dropped us". Without this, `onclose` would always + // arm a reconnect timer — and on a StrictMode mount→unmount→mount cycle + // (or React Fast Refresh) that timer would fire on an orphan hook instance + // and call setState / emit bridge events for a hook that no longer exists, + // racing against the new mount. That race is the original symptom behind + // the "connect → error → disconnect" reconnect loop in issue #7. + const shouldReconnect = useRef(true); const handleEvent = useCallback((event: WsEvent) => { switch (event.type) { @@ -88,27 +96,47 @@ export function useAgentState(): AgentStateHook { } }; - ws.onclose = () => { + ws.onclose = (event) => { + // code/reason help diagnose why the server (or browser) dropped the + // connection — 1009 = message too big, 1006 = abnormal closure, 1000 = + // normal. wasClean=false signals an unclean teardown (issue #7 hunt). + if (!shouldReconnect.current) { + console.log( + `[WS] closed by cleanup code=${event.code} reason="${event.reason}" wasClean=${event.wasClean}`, + ); + return; + } setConnected(false); eventBridge.emit('ws:disconnected'); - console.log('[WS] disconnected, reconnecting in', RECONNECT_DELAY_MS, 'ms'); + console.log( + `[WS] disconnected code=${event.code} reason="${event.reason}" wasClean=${event.wasClean} — reconnecting in`, + RECONNECT_DELAY_MS, + 'ms', + ); reconnectTimer.current = setTimeout(connect, RECONNECT_DELAY_MS); }; ws.onerror = (err) => { - console.error('[WS] error:', err); + // Browsers do not expose error details for security; only the event type + // is meaningful. Pair this with the onclose code/reason for diagnosis. + console.error(`[WS] error type=${err.type}`); ws.close(); }; }, [handleEvent]); useEffect(() => { + shouldReconnect.current = true; connect(); return () => { + // Mark cleanup so the upcoming `onclose` does NOT schedule a reconnect. + shouldReconnect.current = false; if (reconnectTimer.current !== null) { clearTimeout(reconnectTimer.current); + reconnectTimer.current = null; } wsRef.current?.close(); + wsRef.current = null; }; }, [connect]); diff --git a/server/src/index.ts b/server/src/index.ts index 304710f..fe7ee62 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -252,8 +252,8 @@ const server = Bun.serve({ wsServer.handleOpen(ws); wsServer.sendSnapshot(ws, stateManager.getAll(), allConfigDirs()); }, - close(ws: WsClient) { - wsServer.handleClose(ws); + close(ws: WsClient, code: number, reason: string) { + wsServer.handleClose(ws, code, reason); }, message() { // Client-to-server messages not needed in Phase 1 diff --git a/server/src/state/agent-state-manager.test.ts b/server/src/state/agent-state-manager.test.ts index a531c86..d1184d8 100644 --- a/server/src/state/agent-state-manager.test.ts +++ b/server/src/state/agent-state-manager.test.ts @@ -462,6 +462,104 @@ describe('AgentStateManager', () => { expect(agent!.toolCalls).toHaveLength(2); }); + test('caps toolCalls on first event when initial batch exceeds limit (creation path)', () => { + const mgr = new AgentStateManager(); + const bigBatch = Array.from({ length: 55 }, (_, i) => ({ + id: `tc-${i}`, + name: 'Read', + timestamp: Date.now() + i, + input: { file_path: `/file-${i}.ts` }, + })); + mgr.processEvent(makeEvent({ toolCalls: bigBatch })); + + const agent = mgr.getAgent('sess-1'); + expect(agent!.toolCalls).toHaveLength(50); + // Oldest 5 dropped — newest survives + expect(agent!.toolCalls[0]!.id).toBe('tc-5'); + expect(agent!.toolCalls[49]!.id).toBe('tc-54'); + }); + + test('caps toolCalls across accumulated updates (update path drops oldest)', () => { + const mgr = new AgentStateManager(); + // Seed: 40 calls + const initial = Array.from({ length: 40 }, (_, i) => ({ + id: `tc-${i}`, + name: 'Read', + timestamp: Date.now() + i, + input: { file_path: `/file-${i}.ts` }, + })); + mgr.processEvent(makeEvent({ toolCalls: initial })); + + // Update: 15 more calls (total would be 55, cap to 50) + const more = Array.from({ length: 15 }, (_, i) => ({ + id: `tc-extra-${i}`, + name: 'Bash', + timestamp: Date.now() + 100 + i, + input: { command: `cmd-${i}` }, + })); + mgr.processEvent(makeEvent({ toolCalls: more, activity: 'bash' })); + + const agent = mgr.getAgent('sess-1'); + expect(agent!.toolCalls).toHaveLength(50); + // Oldest 5 dropped — first surviving call is the 6th original + expect(agent!.toolCalls[0]!.id).toBe('tc-5'); + // Newest extras at the tail + expect(agent!.toolCalls[49]!.id).toBe('tc-extra-14'); + }); + + test('caps filesModified with dedup, preserving newest after cap', () => { + const mgr = new AgentStateManager(); + // First batch: 30 unique files + const firstBatch = Array.from({ length: 30 }, (_, i) => ({ + id: `tc-${i}`, + name: 'Edit', + timestamp: Date.now() + i, + input: { file_path: `/file-${i}.ts` }, + })); + mgr.processEvent(makeEvent({ toolCalls: firstBatch, activity: 'editing' })); + + // Second batch: duplicates of /file-0 and /file-1 (dedup skip) + // plus 25 new files (cap kicks in) + const secondBatch = [ + { id: 'tc-dup-0', name: 'Edit', timestamp: Date.now() + 100, input: { file_path: '/file-0.ts' } }, + { id: 'tc-dup-1', name: 'Edit', timestamp: Date.now() + 101, input: { file_path: '/file-1.ts' } }, + ...Array.from({ length: 25 }, (_, i) => ({ + id: `tc-new-${i}`, + name: 'Edit', + timestamp: Date.now() + 200 + i, + input: { file_path: `/new-${i}.ts` }, + })), + ]; + mgr.processEvent(makeEvent({ toolCalls: secondBatch, activity: 'editing' })); + + const agent = mgr.getAgent('sess-1'); + // 30 (unique) + 25 (new unique) = 55, capped to 50; duplicates skipped + expect(agent!.filesModified).toHaveLength(50); + // Oldest 5 dropped — first surviving is /file-5.ts + expect(agent!.filesModified[0]).toBe('/file-5.ts'); + // Newest at the tail + expect(agent!.filesModified[49]).toBe('/new-24.ts'); + }); + + test('empty toolCalls in update event preserves existing accumulated history', () => { + const mgr = new AgentStateManager(); + // Seed: 10 calls + const seed = Array.from({ length: 10 }, (_, i) => ({ + id: `tc-${i}`, + name: 'Read', + timestamp: Date.now() + i, + input: { file_path: `/file-${i}.ts` }, + })); + mgr.processEvent(makeEvent({ toolCalls: seed })); + + // Empty update — should not mutate the existing array + mgr.processEvent(makeEvent({ toolCalls: [], activity: 'thinking' })); + + const agent = mgr.getAgent('sess-1'); + expect(agent!.toolCalls).toHaveLength(10); + expect(agent!.toolCalls.map((tc) => tc.id)).toEqual(seed.map((tc) => tc.id)); + }); + test('getAll returns all agents', () => { const mgr = new AgentStateManager(); mgr.processEvent(makeEvent({ sessionId: 'sess-1' })); diff --git a/server/src/state/agent-state-manager.ts b/server/src/state/agent-state-manager.ts index f741fd0..7494aeb 100644 --- a/server/src/state/agent-state-manager.ts +++ b/server/src/state/agent-state-manager.ts @@ -4,6 +4,24 @@ import type { ParsedEvent } from '../parsers/session-parser'; const EDIT_TOOLS = new Set(['Edit', 'Write', 'NotebookEdit']); +// AgentState retention policy: each agent exposes a bounded "recent history" +// of tool calls and modified files to the UI. The Party Bar, Detail Panel, +// and village scene only need the most recent activity — older entries are +// dropped FIFO. Transport-level concerns (payload byte budget, broadcast +// frame size) live in `WebSocketServer`; this cap is the state manager's own +// input policy and does not depend on which transport happens to ship it. +const MAX_TOOL_CALLS_PER_AGENT = 50; +const MAX_FILES_MODIFIED_PER_AGENT = 50; + +function trimAgentHistory(agent: AgentState): void { + if (agent.toolCalls.length > MAX_TOOL_CALLS_PER_AGENT) { + agent.toolCalls.splice(0, agent.toolCalls.length - MAX_TOOL_CALLS_PER_AGENT); + } + if (agent.filesModified.length > MAX_FILES_MODIFIED_PER_AGENT) { + agent.filesModified.splice(0, agent.filesModified.length - MAX_FILES_MODIFIED_PER_AGENT); + } +} + function cwdBasename(cwd: string): string { const parts = cwd.split('/').filter((p) => p.length > 0); return parts[parts.length - 1] ?? cwd; @@ -143,6 +161,7 @@ export class AgentStateManager { if (existing === undefined) { const agent = this.createAgent(event, configDir, source, nameOverride, subagentCtx); + trimAgentHistory(agent); this.agents.set(event.sessionId, agent); this.applyDerivedStatus(agent); this.applyTurnAndError(agent, event); @@ -525,6 +544,7 @@ export class AgentStateManager { agent.filesModified.push(f); } } + trimAgentHistory(agent); } private extractModifiedFiles(event: ParsedEvent): string[] { diff --git a/server/src/ws/websocket-server.ts b/server/src/ws/websocket-server.ts index 599d807..f5bfd3c 100644 --- a/server/src/ws/websocket-server.ts +++ b/server/src/ws/websocket-server.ts @@ -3,6 +3,11 @@ import type { WsEvent, AgentState } from '../types'; export type WsClient = ServerWebSocket<{ id: string }>; +// Warn when a single broadcast frame crosses this size — historically the +// snapshot grew large enough (multi-MB) to make the browser drop the WS on +// connect (issue #7). Keeps an eye on regressions without flooding the log. +const LARGE_MESSAGE_WARN_BYTES = 500 * 1024; + export class WebSocketServer { private clients = new Set(); @@ -11,14 +16,24 @@ export class WebSocketServer { console.log(`[WS] client connected (total: ${this.clients.size})`); } - handleClose(ws: WsClient): void { + handleClose(ws: WsClient, code?: number, reason?: string): void { this.clients.delete(ws); - console.log(`[WS] client disconnected (total: ${this.clients.size})`); + // code/reason help diagnose abnormal disconnects — e.g. 1009 (message too + // big), 1006 (abnormal closure). Empty reason and code 1000 are normal. + const codeStr = code !== undefined ? String(code) : '?'; + const reasonStr = reason !== undefined && reason.length > 0 ? ` reason="${reason}"` : ''; + console.log(`[WS] client disconnected (total: ${this.clients.size}) code=${codeStr}${reasonStr}`); } sendSnapshot(ws: WsClient, agents: AgentState[], configDirs: readonly string[]): void { const event: WsEvent = { type: 'snapshot', agents, configDirs: [...configDirs] }; - ws.send(JSON.stringify(event)); + const data = JSON.stringify(event); + const bytes = Buffer.byteLength(data, 'utf8'); + console.log(`[WS] snapshot bytes=${bytes} agents=${agents.length}`); + if (bytes > LARGE_MESSAGE_WARN_BYTES) { + console.warn(`[WS] large snapshot frame: ${bytes} bytes (warn threshold: ${LARGE_MESSAGE_WARN_BYTES})`); + } + ws.send(data); } broadcastAgentUpdate(agent: AgentState): void { @@ -43,6 +58,10 @@ export class WebSocketServer { private broadcast(event: WsEvent): void { const data = JSON.stringify(event); + const bytes = Buffer.byteLength(data, 'utf8'); + if (bytes > LARGE_MESSAGE_WARN_BYTES) { + console.warn(`[WS] large ${event.type} frame: ${bytes} bytes (warn threshold: ${LARGE_MESSAGE_WARN_BYTES})`); + } for (const client of this.clients) { client.send(data); } From e3a7e833b8aa09ac16fd6af1bc36971c7bef62c9 Mon Sep 17 00:00:00 2001 From: "JeongMinseok(Nalo)" Date: Wed, 20 May 2026 01:33:22 +0900 Subject: [PATCH 05/16] [issues-9] WS onclose: stale-ws identity guard (StrictMode race) (#10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(client): stale-ws guard for onclose during StrictMode race (#9) PR #8's shouldReconnect ref guards the cleanup path but not the StrictMode mount→unmount→mount race: by the time ws_A's async onclose fires, mount B has already flipped the same ref back to true, so the stale ws's reconnect path runs against the freshly-mounted hook. Identity check on wsRef.current is stricter — if the firing socket isn't the current one, it must be a leftover from a previous mount and must not drive a reconnect. Verified in dev: the new "closed (stale ws)" log line fires on the unmount→remount cycle as expected. Note: this fixes one strand of the issue #7 disconnect loop. A separate path remains where onerror fires before any message is received and reconnect runs on the still-current ws — that's tracked separately because the trigger is on the server/network side, not the React lifecycle. * test(client): regression test for stale-ws onclose classifier (#9) Codex senior MEDIUM: the StrictMode race fix in this PR only lives as a guard inside an async onclose handler — if the next refactor restores the old "reconnect on every close" behavior, no test would catch it. Extract the three-way decision (stale / cleanup / reconnect) into a pure `classifyCloseEvent` helper exported from useAgentState. The onclose handler now switches on the verdict, and the helper carries five unit tests covering the exact StrictMode mount/unmount/mount sequence (firing-ws-not-current returns "stale" regardless of the shouldReconnect flag). No behavioral change — the same branches still produce the same logs and same reconnect schedule. The refactor exists for testability. --- client/src/hooks/useAgentState.test.ts | 41 +++++++++++++++++++++++++ client/src/hooks/useAgentState.ts | 42 ++++++++++++++++++++------ 2 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 client/src/hooks/useAgentState.test.ts diff --git a/client/src/hooks/useAgentState.test.ts b/client/src/hooks/useAgentState.test.ts new file mode 100644 index 0000000..12766a0 --- /dev/null +++ b/client/src/hooks/useAgentState.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect } from 'bun:test'; +import { classifyCloseEvent } from './useAgentState'; + +// We compare WebSocket references by identity inside the helper, so the test +// can fake them as opaque objects — none of the WebSocket API is exercised. +function fakeWs(): WebSocket { + return {} as WebSocket; +} + +describe('classifyCloseEvent', () => { + it('returns "reconnect" when the firing socket is the current one and reconnect is enabled', () => { + const ws = fakeWs(); + expect(classifyCloseEvent(ws, ws, true)).toBe('reconnect'); + }); + + it('returns "cleanup" when refs match but the effect has flipped shouldReconnect to false', () => { + const ws = fakeWs(); + expect(classifyCloseEvent(ws, ws, false)).toBe('cleanup'); + }); + + it('returns "stale" when the firing socket is not the one held by the hook (StrictMode race)', () => { + const newWs = fakeWs(); + const oldWs = fakeWs(); + // shouldReconnect re-flipped to true by mount B, but ws_A is firing — the + // identity check must override and skip reconnect. + expect(classifyCloseEvent(newWs, oldWs, true)).toBe('stale'); + }); + + it('returns "stale" when the hook has nulled out its ref (cleanup before unmount finishes)', () => { + const oldWs = fakeWs(); + expect(classifyCloseEvent(null, oldWs, true)).toBe('stale'); + }); + + it('"stale" takes precedence over "cleanup" — identity check runs first', () => { + const newWs = fakeWs(); + const oldWs = fakeWs(); + // Even if shouldReconnect is false (would otherwise be "cleanup"), the + // stale ws still gets the stale verdict because identity check is first. + expect(classifyCloseEvent(newWs, oldWs, false)).toBe('stale'); + }); +}); diff --git a/client/src/hooks/useAgentState.ts b/client/src/hooks/useAgentState.ts index 2eb5f59..e6c6cb9 100644 --- a/client/src/hooks/useAgentState.ts +++ b/client/src/hooks/useAgentState.ts @@ -7,6 +7,30 @@ import { WS_URL } from '../config'; const RECONNECT_DELAY_MS = 3000; const MAX_LOG_ENTRIES = 200; +/** + * Decision the WebSocket onclose handler should take. Extracted into a pure + * function so the StrictMode mount→unmount→mount race can be regression-tested + * without standing up a React render environment. + * + * - `stale` — the firing socket is not the one currently held by the hook. + * A previous mount's WebSocket fired its onclose late after a new mount + * already replaced wsRef.current. Skip reconnect; the new ws owns it. + * - `cleanup` — the effect cleanup intentionally closed the socket. Skip + * reconnect; the hook is unmounting. + * - `reconnect` — genuine server- or network-side close. Schedule reconnect. + */ +export type CloseAction = 'stale' | 'cleanup' | 'reconnect'; + +export function classifyCloseEvent( + currentWs: WebSocket | null, + firingWs: WebSocket, + shouldReconnect: boolean, +): CloseAction { + if (currentWs !== firingWs) return 'stale'; + if (!shouldReconnect) return 'cleanup'; + return 'reconnect'; +} + export interface AgentStateHook { agents: AgentState[]; activityLog: ActivityLogEntry[]; @@ -100,19 +124,19 @@ export function useAgentState(): AgentStateHook { // code/reason help diagnose why the server (or browser) dropped the // connection — 1009 = message too big, 1006 = abnormal closure, 1000 = // normal. wasClean=false signals an unclean teardown (issue #7 hunt). - if (!shouldReconnect.current) { - console.log( - `[WS] closed by cleanup code=${event.code} reason="${event.reason}" wasClean=${event.wasClean}`, - ); + const action = classifyCloseEvent(wsRef.current, ws, shouldReconnect.current); + const closeMeta = `code=${event.code} reason="${event.reason}" wasClean=${event.wasClean}`; + if (action === 'stale') { + console.log(`[WS] closed (stale ws) ${closeMeta} — skipping reconnect`); + return; + } + if (action === 'cleanup') { + console.log(`[WS] closed by cleanup ${closeMeta}`); return; } setConnected(false); eventBridge.emit('ws:disconnected'); - console.log( - `[WS] disconnected code=${event.code} reason="${event.reason}" wasClean=${event.wasClean} — reconnecting in`, - RECONNECT_DELAY_MS, - 'ms', - ); + console.log(`[WS] disconnected ${closeMeta} — reconnecting in`, RECONNECT_DELAY_MS, 'ms'); reconnectTimer.current = setTimeout(connect, RECONNECT_DELAY_MS); }; From d4c80b72f03ea3754f2cfaba87f290063d0eaec0 Mon Sep 17 00:00:00 2001 From: "JeongMinseok(Nalo)" Date: Wed, 20 May 2026 21:05:11 +0900 Subject: [PATCH 06/16] [issues-11] WS diagnostic: log Bun send result + drop onerror.close race (#12) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * diagnostic(ws): tighten onerror cleanup + log Bun send result (#11) The 1006 reconnect loop in dev didn't go away with PR #8 cap or PR #10 stale-ws guard alone — the next strand needs more data from the server side, not just the browser side. Two small changes: * Drop the explicit `ws.close()` inside `onerror`. The WebSocket spec guarantees `onclose` fires after `onerror`, so the explicit close only adds a teardown race against the just-arriving server frame. This is a spec-correctness cleanup; it does not by itself end the loop in our environment. * Log readyState before and after every `sendSnapshot`, plus the return value of `ws.send`. The browser already reports code/reason/wasClean on close; pairing that with the server's view of the socket at send time should triangulate whether Bun thinks the socket was already closing when we tried to push 160KB of snapshot at it. Root cause for the loop itself is still open — this PR just makes the next debugging session cheaper by closing the data gap. * diagnostic(ws): add clientId to open/close/snapshot logs (#11) Codex senior MEDIUM: under StrictMode double-mount or multi-tab usage, the bare `[WS] snapshot bytes=…` log can't be tied back to the matching `client disconnected` line. The server already carries `ws.data.id` from the HTTP upgrade — include it in all three lifecycle log lines so follow-up root-cause work on the 1006 loop has a stable event key. Also expands the `sendResult` comment to make the Bun-specific values explicit (-1 backpressure, 0 dropped, 1+ bytes sent) — that's the mapping the next debugging session needs to read the logs against. --- client/src/hooks/useAgentState.ts | 4 +++- server/src/ws/websocket-server.ts | 22 ++++++++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/client/src/hooks/useAgentState.ts b/client/src/hooks/useAgentState.ts index e6c6cb9..e9838c0 100644 --- a/client/src/hooks/useAgentState.ts +++ b/client/src/hooks/useAgentState.ts @@ -143,8 +143,10 @@ export function useAgentState(): AgentStateHook { ws.onerror = (err) => { // Browsers do not expose error details for security; only the event type // is meaningful. Pair this with the onclose code/reason for diagnosis. + // Do NOT call ws.close() here — the WebSocket spec guarantees onclose + // fires automatically after onerror, so an explicit close just adds a + // teardown race against the freshly-arriving server frame (issue #11). console.error(`[WS] error type=${err.type}`); - ws.close(); }; }, [handleEvent]); diff --git a/server/src/ws/websocket-server.ts b/server/src/ws/websocket-server.ts index f5bfd3c..b0e8f4d 100644 --- a/server/src/ws/websocket-server.ts +++ b/server/src/ws/websocket-server.ts @@ -13,27 +13,41 @@ export class WebSocketServer { handleOpen(ws: WsClient): void { this.clients.add(ws); - console.log(`[WS] client connected (total: ${this.clients.size})`); + console.log(`[WS] client connected clientId=${ws.data.id} (total: ${this.clients.size})`); } handleClose(ws: WsClient, code?: number, reason?: string): void { this.clients.delete(ws); // code/reason help diagnose abnormal disconnects — e.g. 1009 (message too // big), 1006 (abnormal closure). Empty reason and code 1000 are normal. + // clientId pairs the close with the matching open / snapshot send under + // StrictMode double-mount and multi-tab races (issue #11). const codeStr = code !== undefined ? String(code) : '?'; const reasonStr = reason !== undefined && reason.length > 0 ? ` reason="${reason}"` : ''; - console.log(`[WS] client disconnected (total: ${this.clients.size}) code=${codeStr}${reasonStr}`); + console.log( + `[WS] client disconnected clientId=${ws.data.id} (total: ${this.clients.size}) code=${codeStr}${reasonStr}`, + ); } sendSnapshot(ws: WsClient, agents: AgentState[], configDirs: readonly string[]): void { const event: WsEvent = { type: 'snapshot', agents, configDirs: [...configDirs] }; const data = JSON.stringify(event); const bytes = Buffer.byteLength(data, 'utf8'); - console.log(`[WS] snapshot bytes=${bytes} agents=${agents.length}`); + // readyState before and after send pins down whether Bun saw the socket as + // OPEN at send time, and whether send() itself flipped it. sendResult is + // Bun-specific: -1 = backpressure, 0 = dropped, 1+ = bytes sent. clientId + // pairs this with the matching open/close in the log under reconnect + // storms (issue #11). + const readyBefore = ws.readyState; + const sendResult = ws.send(data); + const readyAfter = ws.readyState; + console.log( + `[WS] snapshot clientId=${ws.data.id} bytes=${bytes} agents=${agents.length} ` + + `readyBefore=${readyBefore} sendResult=${sendResult} readyAfter=${readyAfter}`, + ); if (bytes > LARGE_MESSAGE_WARN_BYTES) { console.warn(`[WS] large snapshot frame: ${bytes} bytes (warn threshold: ${LARGE_MESSAGE_WARN_BYTES})`); } - ws.send(data); } broadcastAgentUpdate(agent: AgentState): void { From a231ac9d89f029ae968d6d1c95c1fe4a8c907715 Mon Sep 17 00:00:00 2001 From: "JeongMinseok(Nalo)" Date: Wed, 20 May 2026 21:47:58 +0900 Subject: [PATCH 07/16] [issues-13] BootScene preload diagnostics + negative result for maxParallelDownloads (#14) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * diagnostic(boot): log preload progress + file completes for issue #13 The BootScene → VillageScene transition is silently stalling: progress hits ~30% and then no further filecomplete or 'complete' event fires, so create() never runs, the ws:connected listener is never registered, and the React overlay stays hidden forever. Three listeners on `this.load` make the next debugging session much cheaper: * `loaderror` — logs which file 404'd if the issue turns out to be a silent 404 we never registered for. * `complete` — confirms when (or whether) the loader actually thinks it's done. Today: never fires in the broken environment. * `filecomplete` — names the last file the loader managed to finish before the hang. That filename is the wedge for narrowing the bug. The progress handler also reports `totalToLoad` / `totalComplete` so we can tell if the stall is a download or a decode step. Empirically tried `this.load.maxParallelDownloads = 6` and it made the stall worse — progress no longer leaves 0%, instead of getting to 30% — so that knob is NOT the fix. This PR ships diagnostics only. * diagnostic(boot): use Phaser constants + log totals on every event (#13) Codex senior MEDIUM on PR #14: the diagnostic listeners need three small reinforcements before they're actually useful for hunting the 30%-stuck root cause. * Switch literal event names ('complete', 'filecomplete', 'progress') to the Phaser.Loader.Events constants. Same effect, but it now surfaces if a future Phaser bump renames or removes an event. * `complete` now also logs `totalToLoad / totalComplete / totalFailed`. In a stuck-at-30% report, a stray `complete` event with totalFailed > 0 instantly tells us a silent failure path is winning. * `filecomplete` now logs `src` alongside `key/type`. With a parallel loader, the last file to finish is NOT the stuck one; the stuck file is whatever's still in flight. Knowing the path of the last finisher narrows which batch the stall starts in. * `loaderror` now also logs `key` and `type`, not just `src`. * `progress` fires on either a pct-bucket change OR a totalComplete tick — so a partial-load stall inside a single bucket (queue=60, complete frozen at 18) doesn't get swallowed by the 10% throttle. Still diagnostics-only. Root cause for the loader stall is the next strand. --- client/src/game/scenes/BootScene.ts | 35 +++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/client/src/game/scenes/BootScene.ts b/client/src/game/scenes/BootScene.ts index 40017da..38880fd 100644 --- a/client/src/game/scenes/BootScene.ts +++ b/client/src/game/scenes/BootScene.ts @@ -18,9 +18,44 @@ export class BootScene extends Phaser.Scene { } preload(): void { + // Diagnostic logging for issue #13 — preload stuck at ~30%. These three + // listeners pin down which file ID was the last to land and at what + // progress fraction. Empirically reducing `maxParallelDownloads` makes + // things WORSE (drops to 0% stuck), so this PR ships diagnostics only + // and the real fix is tracked as the next strand. this.load.on(Phaser.Loader.Events.FILE_LOAD_ERROR, (file: Phaser.Loader.File) => { + console.warn(`[BOOT] FILE_LOAD_ERROR src=${file.src} key=${file.key} type=${file.type}`); this.missingAssets.push(file.src); }); + this.load.on(Phaser.Loader.Events.COMPLETE, () => { + console.log( + `[BOOT] preload COMPLETE totalToLoad=${this.load.totalToLoad} totalComplete=${this.load.totalComplete} totalFailed=${this.load.totalFailed}`, + ); + }); + // FILE_COMPLETE fires when an individual asset finishes. Logging both + // key AND src lets the reader pair a 30%-stuck moment with the exact + // path that landed last (the stuck file is in flight, not this one). + this.load.on(Phaser.Loader.Events.FILE_COMPLETE, (key: string, type: string, _data: unknown) => { + const file = this.load.list?.entries?.find?.((f: Phaser.Loader.File) => f.key === key); + const src = file?.src ?? '?'; + console.log(`[BOOT] filecomplete type=${type} key=${key} src=${src}`); + }); + // Log on either a pct bucket change OR a totalComplete tick. The bucket + // alone hides progress within a 10% range; tracking totalComplete makes + // a partial-load stall (e.g. queue=60, complete frozen at 18) obvious. + let lastReportedPct = -1; + let lastReportedComplete = -1; + this.load.on(Phaser.Loader.Events.PROGRESS, (p: number) => { + const pct = Math.floor(p * 10) * 10; + const totalComplete = this.load.totalComplete; + if (pct !== lastReportedPct || totalComplete !== lastReportedComplete) { + lastReportedPct = pct; + lastReportedComplete = totalComplete; + console.log( + `[BOOT] preload progress ${pct}% (totalToLoad=${this.load.totalToLoad}, totalComplete=${totalComplete}, totalFailed=${this.load.totalFailed})`, + ); + } + }); // Logo shown on the loading screen + reused by the React TopBar this.load.image('logo', 'assets/logo.png'); From cbb2f91b09060b92fd3d307d649ab851547f010a Mon Sep 17 00:00:00 2001 From: "JeongMinseok(Nalo)" Date: Thu, 21 May 2026 01:44:38 +0900 Subject: [PATCH 08/16] fix: StrictMode race in Phaser game init + label IA cleanup (#17) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Root cause (closes #15) BootScene preload hung at ~31/83 files because React StrictMode mounts PhaserGame twice (mount → unmount → mount). The naive useRef-per-component pattern let cleanup call `game.destroy(true)` before the second mount created a fresh instance, but Phaser's LoaderPlugin keeps in-flight HTTP requests alive past destroy(). Two Phaser.Game instances then fetched the same ~80-file asset batch in parallel, saturated the browser's HTTP/1.1 connection pool, and stalled preload — `villageReady` never fired, the overlay never mounted, only a black canvas was visible. Verified against baseline (a16e2fb), main (a231ac9), and v0.0.1 (67d92b9): all three reproduced the same hang, ruling out PRs #4 / #8 / #10 / #12 / #14 as the cause and confirming the bug is upstream-inherent. ## Fix `PhaserGame.tsx` anchors the game in a module-level slot. The second StrictMode mount reuses the existing instance and reparents its canvas; cleanup defers destruction (`setTimeout(0)`) so a fast remount can cancel it. The game tears down only on genuine unmount (page navigation). ## Related cleanup - `text.ts` — `addCrispText` now applies NEAREST (was LINEAR, which bilinearly smeared text against the pixel-art tiles). Removed an ill-fated `setRoundPixels(true)` call — Phaser 4 dropped the per-Text method; sub-pixel snap belongs at game-level `roundPixels: true`. - Fonts — Cinzel for headings (building names, Village Gate, PartyBar title) + Fira Code for in-canvas labels (Activity Feed and on-sprite labels share one typography stack). VT323 / Pixelify Sans were tried and rejected: pixel-style faces blur at 10–11px even with NEAREST. - Label readability — replaced stroke + drop shadow with a translucent black plate (`rgba(0,0,0,0.65)` + 4×2 padding) on the activity label, matching the PartyBar pattern. ## Label IA — partial (continues in #16) Started consolidating on-sprite text: only the activity label remains in HeroSprite. Name / subagent / source badge / model / detail / task all move to PartyBar + Detail Panel per the new IA. This commit still leaves broken references in `teleportTo` / `setSelected` / `updateDepth` / `setName` / `layoutSubagentAndSource` / `layoutActivityAndModel` / `setModelBadge` — those, plus the new "Name under feet (2 lines) + speech bubble (task + live activity message) above head + PartyBar index numbers" UI, are tracked in #16. Refs: closes #15, follows up #16 --- client/index.html | 2 +- client/src/game/PhaserGame.tsx | 41 +++++++--- client/src/game/entities/Building.ts | 2 +- client/src/game/entities/HeroSprite.ts | 89 +++------------------- client/src/game/scenes/BootScene.ts | 12 +-- client/src/game/terrain/TerrainRenderer.ts | 6 +- client/src/game/text.ts | 21 ++--- 7 files changed, 64 insertions(+), 109 deletions(-) diff --git a/client/index.html b/client/index.html index 244065f..5818587 100644 --- a/client/index.html +++ b/client/index.html @@ -6,7 +6,7 @@ Agent Quest - +