From f9eb190ef953bbd87207955342a84bdd74d0029f Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Sat, 24 Jan 2026 01:38:58 +0100 Subject: [PATCH] refactor(coding-agent): simplify AgentSession --- .../coding-agent/src/core/agent-session.ts | 44 ++----------------- .../src/modes/interactive/interactive-mode.ts | 6 +-- .../coding-agent/src/modes/rpc/rpc-mode.ts | 4 +- packages/coding-agent/src/utils/sleep.ts | 18 ++++++++ packages/coding-agent/test/sdk-skills.test.ts | 12 ++--- 5 files changed, 33 insertions(+), 51 deletions(-) create mode 100644 packages/coding-agent/src/utils/sleep.ts diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 73e18a02..27054553 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -27,6 +27,7 @@ import { isContextOverflow, modelsAreEqual, supportsXhigh } from "@mariozechner/ import { getAuthPath } from "../config.js"; import { theme } from "../modes/interactive/theme/theme.js"; import { stripFrontmatter } from "../utils/frontmatter.js"; +import { sleep } from "../utils/sleep.js"; import { type BashResult, executeBash as executeBashCommand, executeBashWithOperations } from "./bash-executor.js"; import { type CompactionResult, @@ -62,10 +63,9 @@ import { import type { BashExecutionMessage, CustomMessage } from "./messages.js"; import type { ModelRegistry } from "./model-registry.js"; import { expandPromptTemplate, type PromptTemplate } from "./prompt-templates.js"; -import type { ResourceDiagnostic, ResourceLoader } from "./resource-loader.js"; +import type { ResourceLoader } from "./resource-loader.js"; import type { BranchSummaryEntry, CompactionEntry, NewSessionOptions, SessionManager } from "./session-manager.js"; import type { SettingsManager } from "./settings-manager.js"; -import type { Skill } from "./skills.js"; import { buildSystemPrompt } from "./system-prompt.js"; import type { BashOperations } from "./tools/bash.js"; import { createAllTools } from "./tools/index.js"; @@ -817,7 +817,7 @@ export class AgentSession { const skillName = spaceIndex === -1 ? text.slice(7) : text.slice(7, spaceIndex); const args = spaceIndex === -1 ? "" : text.slice(spaceIndex + 1).trim(); - const skill = this.skills.find((s) => s.name === skillName); + const skill = this.resourceLoader.getSkills().skills.find((s) => s.name === skillName); if (!skill) return text; // Unknown skill, pass through try { @@ -1031,16 +1031,6 @@ export class AgentSession { return this._followUpMessages; } - /** Skills loaded by resource loader */ - get skills(): readonly Skill[] { - return this._resourceLoader.getSkills().skills; - } - - /** Skill loading diagnostics (warnings, errors, collisions) */ - get skillWarnings(): readonly ResourceDiagnostic[] { - return this._resourceLoader.getSkills().diagnostics; - } - get resourceLoader(): ResourceLoader { return this._resourceLoader; } @@ -1212,13 +1202,6 @@ export class AgentSession { return { model: nextModel, thinkingLevel: this.thinkingLevel, isScoped: false }; } - /** - * Get all available models with valid API keys. - */ - async getAvailableModels(): Promise[]> { - return this._modelRegistry.getAvailable(); - } - // ========================================================================= // Thinking Level Management // ========================================================================= @@ -1937,7 +1920,7 @@ export class AgentSession { // Wait with exponential backoff (abortable) this._retryAbortController = new AbortController(); try { - await this._sleep(delayMs, this._retryAbortController.signal); + await sleep(delayMs, this._retryAbortController.signal); } catch { // Aborted during sleep - emit end event so UI can clean up const attempt = this._retryAttempt; @@ -1964,25 +1947,6 @@ export class AgentSession { return true; } - /** - * Sleep helper that respects abort signal. - */ - private _sleep(ms: number, signal?: AbortSignal): Promise { - return new Promise((resolve, reject) => { - if (signal?.aborted) { - reject(new Error("Aborted")); - return; - } - - const timeout = setTimeout(resolve, ms); - - signal?.addEventListener("abort", () => { - clearTimeout(timeout); - reject(new Error("Aborted")); - }); - }); - } - /** * Cancel in-progress retry. */ diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index 32f23e84..19df0b74 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -347,7 +347,7 @@ export class InteractiveMode { this.skillCommands.clear(); const skillCommandList: SlashCommand[] = []; if (this.settingsManager.getEnableSkillCommands()) { - for (const skill of this.session.skills) { + for (const skill of this.session.resourceLoader.getSkills().skills) { const commandName = `skill:${skill.name}`; this.skillCommands.set(commandName, skill.filePath); skillCommandList.push({ name: commandName, description: skill.description }); @@ -839,7 +839,7 @@ export class InteractiveMode { this.chatContainer.addChild(new Spacer(1)); } - const skills = this.session.skills; + const skills = this.session.resourceLoader.getSkills().skills; if (skills.length > 0) { const skillPaths = skills.map((s) => s.filePath); const groups = this.groupPathsBySource(skillPaths, metadata); @@ -848,7 +848,7 @@ export class InteractiveMode { this.chatContainer.addChild(new Spacer(1)); } - const skillDiagnostics = this.session.skillWarnings; + const skillDiagnostics = this.session.resourceLoader.getSkills().diagnostics; if (skillDiagnostics.length > 0) { const warningLines = this.formatDiagnostics(skillDiagnostics, metadata); this.chatContainer.addChild(new Text(`${theme.fg("warning", "[Skill conflicts]")}\n${warningLines}`, 0, 0)); diff --git a/packages/coding-agent/src/modes/rpc/rpc-mode.ts b/packages/coding-agent/src/modes/rpc/rpc-mode.ts index a01a3c95..9439b2ef 100644 --- a/packages/coding-agent/src/modes/rpc/rpc-mode.ts +++ b/packages/coding-agent/src/modes/rpc/rpc-mode.ts @@ -365,7 +365,7 @@ export async function runRpcMode(session: AgentSession): Promise { // ================================================================= case "set_model": { - const models = await session.getAvailableModels(); + const models = await session.modelRegistry.getAvailable(); const model = models.find((m) => m.provider === command.provider && m.id === command.modelId); if (!model) { return error(id, "set_model", `Model not found: ${command.provider}/${command.modelId}`); @@ -383,7 +383,7 @@ export async function runRpcMode(session: AgentSession): Promise { } case "get_available_models": { - const models = await session.getAvailableModels(); + const models = await session.modelRegistry.getAvailable(); return success(id, "get_available_models", { models }); } diff --git a/packages/coding-agent/src/utils/sleep.ts b/packages/coding-agent/src/utils/sleep.ts new file mode 100644 index 00000000..948f93c4 --- /dev/null +++ b/packages/coding-agent/src/utils/sleep.ts @@ -0,0 +1,18 @@ +/** + * Sleep helper that respects abort signal. + */ +export function sleep(ms: number, signal?: AbortSignal): Promise { + return new Promise((resolve, reject) => { + if (signal?.aborted) { + reject(new Error("Aborted")); + return; + } + + const timeout = setTimeout(resolve, ms); + + signal?.addEventListener("abort", () => { + clearTimeout(timeout); + reject(new Error("Aborted")); + }); + }); +} diff --git a/packages/coding-agent/test/sdk-skills.test.ts b/packages/coding-agent/test/sdk-skills.test.ts index 9f604b10..2d5d86d6 100644 --- a/packages/coding-agent/test/sdk-skills.test.ts +++ b/packages/coding-agent/test/sdk-skills.test.ts @@ -45,8 +45,8 @@ This is a test skill. }); // Skills should be discovered and exposed on the session - expect(session.skills.length).toBeGreaterThan(0); - expect(session.skills.some((s) => s.name === "test-skill")).toBe(true); + expect(session.resourceLoader.getSkills().skills.length).toBeGreaterThan(0); + expect(session.resourceLoader.getSkills().skills.some((s) => s.name === "test-skill")).toBe(true); }); it("should have empty skills when resource loader returns none (--no-skills)", async () => { @@ -69,8 +69,8 @@ This is a test skill. resourceLoader, }); - expect(session.skills).toEqual([]); - expect(session.skillWarnings).toEqual([]); + expect(session.resourceLoader.getSkills().skills).toEqual([]); + expect(session.resourceLoader.getSkills().diagnostics).toEqual([]); }); it("should use provided skills when resource loader supplies them", async () => { @@ -101,7 +101,7 @@ This is a test skill. resourceLoader, }); - expect(session.skills).toEqual([customSkill]); - expect(session.skillWarnings).toEqual([]); + expect(session.resourceLoader.getSkills().skills).toEqual([customSkill]); + expect(session.resourceLoader.getSkills().diagnostics).toEqual([]); }); });