Current section

Files

Jump to
codex_sdk docs 20251214 git_diff_analysis changes-summary.md
Raw

docs/20251214/git_diff_analysis/changes-summary.md

# Git Pull Changes Analysis (a2c86e5d8..5d77d4db6)
## Overview
This document analyzes the changes pulled from the upstream codex repository on 2025-12-14.
**Commit**: `5d77d4db6` - "Reimplement skills loading using SkillsManager + skills/list op. (#7914)"
## Summary
This is a **major architectural refactor** of the skills system, moving from static load-on-startup to dynamic, cacheable, manager-based loading.
## Files Modified (29 total)
### Core Changes
| File | Changes | Description |
|------|---------|-------------|
| `core/src/skills/loader.rs` | +51, -26 | Refactored for roots-based loading |
| `core/src/skills/manager.rs` | **NEW** +48 | New SkillsManager class |
| `core/src/skills/mod.rs` | +2 | Export manager |
| `core/src/skills/model.rs` | +3 | Added SkillScope to SkillMetadata |
| `core/src/codex.rs` | +128, -109 | Major refactor for skills manager |
| `core/src/conversation_manager.rs` | +38, -6 | New skills_manager field & methods |
| `core/src/auth.rs` | +13 | New test helper with custom codex_home |
| `core/src/state/service.rs` | +4, -1 | Field type change |
### Protocol Changes
| File | Changes | Description |
|------|---------|-------------|
| `protocol/src/protocol.rs` | +40, -18 | Op::ListSkills, ListSkillsResponseEvent |
| `app-server-protocol/common.rs` | +4 | Register `skills/list` request/response |
| `app-server-protocol/v2.rs` | +84 | skills/list types (params/response + skill metadata) |
### App-Server Changes
| File | Changes | Description |
|------|---------|-------------|
| `app-server/codex_message_processor.rs` | +67 | skills/list handler |
| `app-server/README.md` | +1 | Document new endpoint |
### UI Changes
| File | Changes | Description |
|------|---------|-------------|
| `tui/src/app.rs` | +33, -33 | ListSkillsResponse handling |
| `tui/src/chatwidget.rs` | +42, -19 | errors_for_cwd() helper |
| `tui2/src/app.rs` | +33, -33 | Same as tui |
| `tui2/src/chatwidget.rs` | +50, -28 | Same as tui |
### Test Changes
| File | Changes | Description |
|------|---------|-------------|
| `core/tests/suite/skills.rs` | +23, -5 | Updated for Op::ListSkills |
| `core/tests/common/test_codex.rs` | +7, -4 | codex_home parameter |
| `core/tests/suite/client.rs` | +34, -19 | Skills manager injection |
## New Features Introduced
### 1. SkillsManager Class
**Location**: `codex-rs/core/src/skills/manager.rs`
```rust
pub struct SkillsManager {
codex_home: PathBuf,
cache_by_cwd: RwLock<HashMap<PathBuf, SkillLoadOutcome>>,
}
```
**Purpose**: Centralized skills management with per-working-directory caching.
**Key Methods**:
- `new(codex_home: PathBuf)` - Constructor
- `skills_for_cwd(&self, cwd: &Path) -> SkillLoadOutcome` - Cached skill discovery
### 2. Op::ListSkills Operation
**Location**: `protocol/src/protocol.rs`
```rust
Op::ListSkills {
cwds: Vec<PathBuf>, // Empty = use session default
}
```
**Purpose**: Client-to-server request for skills discovery.
### 3. ListSkillsResponseEvent
```rust
pub struct ListSkillsResponseEvent {
pub skills: Vec<SkillsListEntry>,
}
pub struct SkillsListEntry {
pub cwd: PathBuf,
pub skills: Vec<SkillMetadata>,
pub errors: Vec<SkillErrorInfo>,
}
```
### 4. SkillScope Enum
```rust
pub enum SkillScope {
User, // From ~/.codex/skills
Repo, // From .codex/skills in git root
}
```
## API Breaking Changes
### Codex::spawn() Signature
**Before**:
```rust
pub async fn spawn(
config: Config,
auth_manager: Arc<AuthManager>,
models_manager: Arc<ModelsManager>,
conversation_history: InitialHistory,
session_source: SessionSource,
) -> CodexResult<CodexSpawnOk>
```
**After**:
```rust
pub async fn spawn(
config: Config,
auth_manager: Arc<AuthManager>,
models_manager: Arc<ModelsManager>,
skills_manager: Arc<SkillsManager>, // NEW PARAMETER
conversation_history: InitialHistory,
session_source: SessionSource,
) -> CodexResult<CodexSpawnOk>
```
### SessionServices Field
**Before**: `skills: Option<SkillLoadOutcome>`
**After**: `skills_manager: Arc<SkillsManager>`
### SessionConfiguredEvent
**Before**: Included `skill_load_outcome: Option<SkillLoadOutcomeInfo>`
**After**: Field removed - clients must use `Op::ListSkills`
## Behavioral Changes
| Aspect | Before | After |
|--------|--------|-------|
| Skills Loading | Static, once at session start | Dynamic, on-demand per-cwd |
| Caching | No caching | Per-cwd caching in SkillsManager |
| Architecture | Skills in SessionConfiguredEvent | Separate Op::ListSkills + response |
| Scope Tracking | No user/repo distinction | SkillScope enum distinguishes |
| Multiple CWDs | Not supported | ListSkills accepts Vec<PathBuf> |
| API Style | Event-driven push | RPC-style request/response |
## Porting Implications for Elixir
1. **New Protocol Types Required**:
- `Op.ListSkills` operation
- `ListSkillsResponseEvent` event
- `SkillsListEntry` struct
- `SkillScope` enum
2. **If Using Core Protocol**:
- Must handle new event types
- Must implement Op::ListSkills submission
- Must update event handlers
3. **If Using Exec JSONL** (current):
- Skills not exposed via exec transport
- Would need transport change to support