feat(guid+acp): model pre-selection, Codex ACP migration, page refactor & skeleton

[kaizhou-lab]

Closes #931

Summary

This PR improves the Guid (home) page across four areas:

1. ACP model pre-selection — cache model lists from ACP agents, show a model dropdown on Guid page
2. Codex → ACP migration — route new Codex sessions through ACP protocol instead of the custom MCP adapter
3. Bug fixes — fix mode loss on Guid→conversation navigation; improve model switching reliability
4. Page refactor & skeleton — split monolithic 700-line component into modules; add shimmer placeholders to eliminate layout shift
5. Skills loading fix — skip optional skills for non-preset agents; only load skills when the corresponding assistant is enabled

---

1. ACP Model Pre-selection

After the first conversation with an ACP agent, its model list is cached locally. On subsequent visits, the Guid page shows a model dropdown pre-populated from cache. The selected model carries through to the conversation page.

• First use (no cache): shows "Use CLI model" — behavior unchanged
• After one conversation: model dropdown appears with cached options
• Pre-selected model is re-asserted before each prompt via session/set_model

File	Change
src/common/storage.ts	Add acp.cachedModels type definition
src/common/ipcBridge.ts	Add currentModelId to ICreateConversationParams
src/process/task/AcpAgentManager.ts	Add cacheModelList(), called after agent.start()
src/process/initAgent.ts	Pass currentModelId to ACP conversation extra
src/renderer/components/AcpModelSelector.tsx	Add backend/initialModelId props, fallback to cached models
src/agent/acp/index.ts	Track userModelOverride, re-assert before each prompt, inject model switch notice for Claude
src/agent/acp/AcpConnection.ts	Prefer set_model, eagerly update configOptions cache, fix event name typo
src/agent/acp/AcpAdapter.ts	Handle config_option_update event

Why special handling for Claude backend? (env_info cache issue)

When switching models mid-conversation via set_model, the API call correctly uses the new model, but the AI incorrectly self-identifies as the old model. For example, after switching from Opus to Haiku:

• API requests correctly go to Haiku ✅
• But AI still says "I am Claude Opus" ❌

Root cause: Claude Code CLI caches the env_info system prompt section (which contains "You are powered by {model}") with cacheBreak: false. Once computed on the first prompt, it's never recomputed for the session lifetime — regardless of model switches.

// vc() = cacheBreak: false → cached, never recomputed
vc("env_info", () => CB8(model, K))

Neither /model nor set_model clears this cache. It's only cleared by /clear (destroys conversation) or conversation compaction (automatic for long conversations).

Our workaround: inject a <system-reminder> into the next user message after a model switch, telling the AI its model has changed. This mirrors what the terminal /model command prints to stdout.

This is a Claude Code CLI limitation, not an AionUI bug. Identical across CLI versions 2.1.45–2.1.50.

---

2. Codex → ACP Migration

This PR: route new Codex sessions through ACP. Future cleanup: remove old MCP adapter code once migration is validated.

New Codex sessions are created as type: 'acp', backend: 'codex' instead of type: 'codex', routed through the existing ACP infrastructure via the @zed-industries/codex-acp bridge.

Old type: 'codex' history sessions still work — CodexAgentManager and all codex-specific code are preserved for backward compatibility.

Before:  Guid → type: 'codex'  → CodexAgentManager → CodexAgent → MCP → codex CLI
After:   Guid → type: 'acp'    → AcpAgentManager   → AcpConnection → codex-acp → codex CLI

File	Change
src/agent/acp/AcpConnection.ts	Add connectCodex() (NPX-based, @zed-industries/codex-acp@0.9.4); fix absolute cwd for codex
src/types/acpTypes.ts	Add 'codex' to ACP_ROUTED_PRESET_TYPES; update codex backend config

codex-acp capabilities (verified)

Feature	Supported
session/new returns models + configOptions + modes	✅
session/set_model	✅
session/set_config_option	✅
Model format	model_id or model_id/reasoning_effort (e.g. o3/high)
Absolute cwd requirement	✅ (error: "cwd is not absolute")

What's preserved for future cleanup

• src/agent/codex/ directory, CodexAgentManager, codex conversation components, codex IPC bridges
• codexModels.ts (DEFAULT_CODEX_MODELS) — still used by CodexAgentManager for old sessions

---

3. Bug Fixes

Mode loss on Guid→conversation navigation

OpenCode's mode list is build / plan — no default entry. Before the first message, AcpAgentManager doesn't exist, so getMode IPC returns { mode: 'default', initialized: false }. The AgentModeSelector then overwrites the user's Guid selection (e.g. plan) with 'default', which doesn't match any option.

Fix: thread sessionMode from conversation extra to AgentModeSelector as initialMode; skip IPC overwrite when initialized === false.

File	Change
src/renderer/components/AgentModeSelector.tsx	Skip getMode overwrite when uninitialized
src/renderer/pages/conversation/ChatConversation.tsx	Pass sessionMode to AcpChat
src/renderer/pages/conversation/acp/AcpChat.tsx	Forward sessionMode prop
src/renderer/pages/conversation/acp/AcpSendBox.tsx	Pass sessionMode as initialMode

---

4. Guid Page Refactor & Skeleton Loading

Refactor

Split the monolithic 700-line GuidPage into modular hooks, components, and utilities:

Category	Modules
Hooks	useGuidAgentSelection, useGuidInput, useGuidMention, useGuidModelSelection, useGuidSend, useTypewriterPlaceholder
Components	AgentPillBar, AssistantSelectionArea, GuidActionRow, GuidInputCard, GuidModelSelector, GuidSkeleton, MentionDropdown, MentionSelectorBadge, PresetAgentTag, QuickActionButtons
Utils	guidUtils.ts (conversation creation, agent resolution)
Types	types.ts, constants.ts

Skeleton loading

Shimmer placeholders eliminate layout shift while async data loads:

• AgentPillBarSkeleton — 1 wide selected pill + 4 circles, shown while availableAgents === undefined
• AssistantsSkeleton — 3 pill-shaped placeholders (80/100/90px), shown while custom agents load
• Uses === undefined to distinguish "loading" vs "loaded but empty"
• Arco Design semantic colors (--color-fill-2/--color-fill-3) for automatic dark/light theme support

File	Change
src/renderer/pages/guid/GuidPage.tsx	Orchestrates hooks/components; conditionally renders skeletons
src/renderer/pages/guid/index.module.css	Add @keyframes guid-shimmer, .skeleton, .skeletonPill
src/renderer/pages/guid/components/GuidSkeleton.tsx	New: skeleton components
src/renderer/pages/guid/components/*.tsx	New: extracted UI components
src/renderer/pages/guid/hooks/*.ts	New: extracted hooks
src/renderer/pages/guid/utils/guidUtils.ts	New: utility functions
src/renderer/pages/guid/types.ts	New: shared types
src/renderer/pages/guid/constants.ts	New: constants

---

5. Skills Loading Fix

Previously, when using a non-preset agent (e.g. directly selecting Claude or Gemini), all optional skills from the skills/ directory were loaded. Now optional skills are only loaded when the corresponding assistant is enabled and has enabledSkills configured. Builtin skills (_builtin/cron) remain always available.

File	Change
src/process/task/AcpSkillManager.ts	Early return in discoverSkills() when enabledSkills is undefined/empty — only builtin skills loaded
src/agent/gemini/cli/config.ts	Skip loadSkillsFromDir() entirely when enabledSkills is not specified
src/agent/gemini/index.ts	Clear all SkillManager skills after initialize() when enabledSkills is not specified (aioncli-core rediscovers all on init)

---

Test plan

Model pre-selection

• First use of ACP agent: Guid shows "Use CLI model"
• After one conversation, return to Guid: model dropdown appears with cached models
• Select model → send → conversation uses pre-selected model
• Conversation page model selector shows selection immediately (before first message)
• Switch model mid-conversation → ask "what model are you?" → AI identifies correctly
• Restart app: cached model list persists

Mode preservation

• OpenCode: select plan on Guid → conversation shows Plan (not empty/reset)
• Claude/other agents: mode carries through normally

Codex ACP

• Select Codex on Guid → model dropdown shows codex-acp models → creates type: 'acp' conversation
• Old type: 'codex' history sessions still open and work

Skeleton loading

• Guid page opens with shimmer placeholders → agents load → seamless replacement, no layout jump

Skills loading

• Non-preset agent (direct Claude/Gemini): no optional skills injected, only builtin cron available
• Preset assistant (e.g. Cowork): only configured enabledSkills are loaded
• Builtin _builtin/cron skill always available regardless of agent type

General

• npm run lint passes with 0 errors

[github-actions]

Code Review

CRITICAL Issues

1. acp.cachedModels storage type is incorrect (will break cached model lookup)

File: src/common/storage.ts:46-52

// Cached model lists per ACP backend for Guid page pre-selection
'acp.cachedModels'?: Record<string, import('@/types/acpTypes').AcpModelInfo>;

Problem: The PR stores one AcpModelInfo per backend (see cached[this.options.backend] = modelInfo), so the type must be Record<string, AcpModelInfo>. That part is fine. The issue is that AcpModelInfo is a single backend’s model info object, but the code in the renderer treats ConfigStorage.get('acp.cachedModels') as Record<string, AcpModelInfo> and then indexes by backend key. With the current type, this compiles, but it’s very likely wrong at runtime because AcpModelInfo typically contains arrays and flags, and you’re persisting it directly. If AcpModelInfo includes non-serializable fields (common for “info” objects), the storage layer may drop/alter them, causing availableModels to be missing and the UI to crash on .length access.

This is production-breaking if availableModels becomes undefined after persistence, because multiple places do:

• modelInfo.availableModels.length > 0
• currentAcpCachedModelInfo.availableModels.map(...)

Fix: Persist a storage-safe DTO type instead of AcpModelInfo, and validate shape on read.

Example:

// src/types/acpTypes.ts (or a new file)
export type AcpCachedModelInfo = {
  source: 'models';
  currentModelId?: string;
  currentModelLabel?: string;
  canSwitch: boolean;
  availableModels: Array<{ id: string; label: string }>;
};

// src/common/storage.ts
'acp.cachedModels'?: Record<string, import('@/types/acpTypes').AcpCachedModelInfo>;

And in both writer/reader, guard:

if (cachedInfo?.availableModels?.length) { ... }

---

HIGH Issues

2. New console.error added in renderer (violates debug hygiene / lint expectations)

File: src/renderer/pages/guid/index.tsx:579-586

ConfigStorage.get('acp.cachedModels')
  .then((cached) => {
    if (!isActive) return;
    setAcpCachedModels(cached || {});
  })
  .catch((error) => {
    console.error('Failed to load cached ACP models:', error);
  });

Problem: This PR introduces a new console.error in production renderer code. Your ESLint rule says no-console: warn and the PR test plan explicitly expects npm run lint to pass with 0 errors; even if it’s “warn”, this is still a regression in debug hygiene and will likely be flagged in review/CI expectations.

Fix: Replace with the project’s logging/toast mechanism or silently ignore (consistent with AcpModelSelector), e.g.:

.catch(() => {
  // Silently ignore - cached models are optional
});

or use a centralized logger if the project has one.

---

3. Immutability violation when updating cached models object before persisting

File: src/process/task/AcpAgentManager.ts:724-733

const cached = (await ProcessConfig.get('acp.cachedModels')) || {};
cached[this.options.backend] = modelInfo;
await ProcessConfig.set('acp.cachedModels', cached);

Problem: This mutates the cached object in-place. In this project, immutability is a critical convention (“always create new objects, never mutate”). Even though this is in main process config code, it still risks subtle bugs if ProcessConfig.get() returns a shared reference (or if future code keeps references).

Fix: Create a new object:

const cached = (await ProcessConfig.get('acp.cachedModels')) || {};
await ProcessConfig.set('acp.cachedModels', {
  ...cached,
  [this.options.backend]: modelInfo,
});

---

MEDIUM Issues

4. Cached model fallback in AcpModelSelector unnecessarily requires initialModelId

File: src/renderer/components/AcpModelSelector.tsx:33-74

The fallback only runs when both backend && initialModelId are present:

} else if (backend && initialModelId) {
  void loadCachedModelInfo(backend, initialModelId, cancelled);
}

Recommendation: If the goal is “show cached model dropdown for ACP agents (when available)”, you can still show the dropdown even without a preselected model ID by using cachedInfo.currentModelId (if present) or leaving selection empty. This would make the feature more robust for conversations created without currentModelId (older history, migrations, etc.).

Concrete adjustment:

• Trigger cached load when backend exists (not necessarily initialModelId)
• Inside loadCachedModelInfo, choose modelId ?? cachedInfo.currentModelId

---

Summary

Level	Count
CRITICAL	1
HIGH	2
MEDIUM	1

---

🤖 This review was generated by AI and may contain inaccuracies. Please focus on issues you agree with and feel free to disregard any that seem incorrect. Thank you for your contribution!

Note: The following inputs were truncated due to size limits: file contents (>80K chars).
Please review the omitted portions manually.

[kaizhou-lab]

Code Review Response

Thanks for the thorough review! All 4 issues have been addressed in commit ba6a2a97.

1. CRITICAL — availableModels deserialization safety

AcpModelInfo is a plain data interface (strings, booleans, Array<{id: string; label: string}>) — fully JSON-serializable with no non-serializable fields. However, added defensive ?. optional chaining on all availableModels access points as a safeguard against unexpected storage corruption:

• AcpAgentManager.ts:339 — modelInfo.availableModels?.length
• guid/index.tsx:1775 — currentAcpCachedModelInfo.availableModels?.length
• AcpModelSelector.tsx:67 — cachedInfo?.availableModels?.length

Chose not to introduce a separate DTO type since AcpModelInfo is already a pure data contract with no methods or non-serializable members.

2. HIGH — console.error in renderer

Replaced with silent catch, consistent with AcpModelSelector pattern:

.catch(() => {
  // Silently ignore - cached models are optional
});

3. HIGH — Immutability violation in cacheModelList()

Fixed with immutable spread:

const cached = (await ProcessConfig.get('acp.cachedModels')) || {};
await ProcessConfig.set('acp.cachedModels', {
  ...cached,
  [this.options.backend]: modelInfo,
});

4. MEDIUM — Cached model fallback without initialModelId

loadCachedModelInfo now only requires backend. Falls back to cachedInfo.currentModelId when initialModelId is not provided, making it work for older conversations and migrations:

const effectiveModelId = initialModelId || cachedInfo.currentModelId || null;