Fix --no-skills flag not preventing skills from loading

The --no-skills flag set options.skills = [] in main.ts, but the
interactive mode UI would rediscover skills anyway because it called
loadSkills() directly instead of using the already-loaded skills.

Changes:
- Add AgentSession.skills and AgentSession.skillWarnings properties
- discoverSkills() now returns { skills, warnings } instead of Skill[]
- Interactive mode uses session.skills instead of calling loadSkills()
- Update SDK docs and examples for new return type

Fixes #577
This commit is contained in:
Mario Zechner 2026-01-08 23:32:57 +01:00
parent c865ec1d19
commit af2d8509e6
7 changed files with 88 additions and 54 deletions

View file

@ -5,6 +5,7 @@
### Breaking Changes ### Breaking Changes
- `before_agent_start` event now receives `systemPrompt` in the event object and returns `systemPrompt` (full replacement) instead of `systemPromptAppend`. Extensions that were appending must now use `event.systemPrompt + extra` pattern. ([#575](https://github.com/badlogic/pi-mono/issues/575)) - `before_agent_start` event now receives `systemPrompt` in the event object and returns `systemPrompt` (full replacement) instead of `systemPromptAppend`. Extensions that were appending must now use `event.systemPrompt + extra` pattern. ([#575](https://github.com/badlogic/pi-mono/issues/575))
- `discoverSkills()` now returns `{ skills: Skill[], warnings: SkillWarning[] }` instead of `Skill[]`. This allows callers to handle skill loading warnings. ([#577](https://github.com/badlogic/pi-mono/pull/577) by [@cv](https://github.com/cv))
### Added ### Added
@ -17,6 +18,7 @@
- `ssh.ts` example: remote tool execution via `--ssh user@host:/path` - `ssh.ts` example: remote tool execution via `--ssh user@host:/path`
- Wayland clipboard support for `/copy` command using wl-copy with xclip/xsel fallback ([#570](https://github.com/badlogic/pi-mono/pull/570) by [@OgulcanCelik](https://github.com/OgulcanCelik)) - Wayland clipboard support for `/copy` command using wl-copy with xclip/xsel fallback ([#570](https://github.com/badlogic/pi-mono/pull/570) by [@OgulcanCelik](https://github.com/OgulcanCelik))
- **Experimental:** `ctx.ui.custom()` now accepts `{ overlay: true }` option for floating modal components that composite over existing content without clearing the screen ([#558](https://github.com/badlogic/pi-mono/pull/558) by [@nicobailon](https://github.com/nicobailon)) - **Experimental:** `ctx.ui.custom()` now accepts `{ overlay: true }` option for floating modal components that composite over existing content without clearing the screen ([#558](https://github.com/badlogic/pi-mono/pull/558) by [@nicobailon](https://github.com/nicobailon))
- `AgentSession.skills` and `AgentSession.skillWarnings` properties to access loaded skills without rediscovery ([#577](https://github.com/badlogic/pi-mono/pull/577) by [@cv](https://github.com/cv))
### Fixed ### Fixed
@ -26,6 +28,7 @@
- Abort messages now show correct retry attempt count (e.g., "Aborted after 2 retry attempts") ([#568](https://github.com/badlogic/pi-mono/pull/568) by [@tmustier](https://github.com/tmustier)) - Abort messages now show correct retry attempt count (e.g., "Aborted after 2 retry attempts") ([#568](https://github.com/badlogic/pi-mono/pull/568) by [@tmustier](https://github.com/tmustier))
- Fixed Antigravity provider returning 429 errors despite available quota ([#571](https://github.com/badlogic/pi-mono/pull/571) by [@ben-vargas](https://github.com/ben-vargas)) - Fixed Antigravity provider returning 429 errors despite available quota ([#571](https://github.com/badlogic/pi-mono/pull/571) by [@ben-vargas](https://github.com/ben-vargas))
- Fixed malformed thinking text in Gemini/Antigravity responses where thinking content appeared as regular text or vice versa. Cross-model conversations now properly convert thinking blocks to plain text. ([#561](https://github.com/badlogic/pi-mono/issues/561)) - Fixed malformed thinking text in Gemini/Antigravity responses where thinking content appeared as regular text or vice versa. Cross-model conversations now properly convert thinking blocks to plain text. ([#561](https://github.com/badlogic/pi-mono/issues/561))
- `--no-skills` flag now correctly prevents skills from loading in interactive mode ([#577](https://github.com/badlogic/pi-mono/pull/577) by [@cv](https://github.com/cv))
## [0.38.0] - 2026-01-08 ## [0.38.0] - 2026-01-08

View file

@ -528,7 +528,7 @@ eventBus.on("my-extension:status", (data) => console.log(data));
import { createAgentSession, discoverSkills, type Skill } from "@mariozechner/pi-coding-agent"; import { createAgentSession, discoverSkills, type Skill } from "@mariozechner/pi-coding-agent";
// Discover and filter // Discover and filter
const allSkills = discoverSkills(); const { skills: allSkills, warnings } = discoverSkills();
const filtered = allSkills.filter(s => s.name.includes("search")); const filtered = allSkills.filter(s => s.name.includes("search"));
// Custom skill // Custom skill
@ -550,7 +550,7 @@ const { session } = await createAgentSession({
}); });
// Discovery with settings filter // Discovery with settings filter
const skills = discoverSkills(process.cwd(), undefined, { const { skills } = discoverSkills(process.cwd(), undefined, {
ignoredSkills: ["browser-*"], // glob patterns to exclude ignoredSkills: ["browser-*"], // glob patterns to exclude
includeSkills: ["search-*"], // glob patterns to include (empty = all) includeSkills: ["search-*"], // glob patterns to include (empty = all)
}); });
@ -747,7 +747,7 @@ const model = modelRegistry.find("provider", "id"); // Find specific model
const builtIn = getModel("anthropic", "claude-opus-4-5"); // Built-in only const builtIn = getModel("anthropic", "claude-opus-4-5"); // Built-in only
// Skills // Skills
const skills = discoverSkills(cwd, agentDir, skillsSettings); const { skills, warnings } = discoverSkills(cwd, agentDir, skillsSettings);
// Hooks (async - loads TypeScript) // Hooks (async - loads TypeScript)
// Pass eventBus to share pi.events across hooks/tools // Pass eventBus to share pi.events across hooks/tools

View file

@ -8,11 +8,14 @@
import { createAgentSession, discoverSkills, SessionManager, type Skill } from "@mariozechner/pi-coding-agent"; import { createAgentSession, discoverSkills, SessionManager, type Skill } from "@mariozechner/pi-coding-agent";
// Discover all skills from cwd/.pi/skills, ~/.pi/agent/skills, etc. // Discover all skills from cwd/.pi/skills, ~/.pi/agent/skills, etc.
const allSkills = discoverSkills(); const { skills: allSkills, warnings } = discoverSkills();
console.log( console.log(
"Discovered skills:", "Discovered skills:",
allSkills.map((s) => s.name), allSkills.map((s) => s.name),
); );
if (warnings.length > 0) {
console.log("Warnings:", warnings);
}
// Filter to specific skills // Filter to specific skills
const filteredSkills = allSkills.filter((s) => s.name.includes("browser") || s.name.includes("search")); const filteredSkills = allSkills.filter((s) => s.name.includes("browser") || s.name.includes("search"));

View file

@ -50,6 +50,7 @@ import type { ModelRegistry } from "./model-registry.js";
import { expandPromptTemplate, type PromptTemplate } from "./prompt-templates.js"; import { expandPromptTemplate, type PromptTemplate } from "./prompt-templates.js";
import type { BranchSummaryEntry, CompactionEntry, NewSessionOptions, SessionManager } from "./session-manager.js"; import type { BranchSummaryEntry, CompactionEntry, NewSessionOptions, SessionManager } from "./session-manager.js";
import type { SettingsManager, SkillsSettings } from "./settings-manager.js"; import type { SettingsManager, SkillsSettings } from "./settings-manager.js";
import type { Skill, SkillWarning } from "./skills.js";
import type { BashOperations } from "./tools/bash.js"; import type { BashOperations } from "./tools/bash.js";
/** Session-specific events that extend the core AgentEvent */ /** Session-specific events that extend the core AgentEvent */
@ -77,6 +78,10 @@ export interface AgentSessionConfig {
promptTemplates?: PromptTemplate[]; promptTemplates?: PromptTemplate[];
/** Extension runner (created in sdk.ts with wrapped tools) */ /** Extension runner (created in sdk.ts with wrapped tools) */
extensionRunner?: ExtensionRunner; extensionRunner?: ExtensionRunner;
/** Loaded skills (already discovered by SDK) */
skills?: Skill[];
/** Skill loading warnings (already captured by SDK) */
skillWarnings?: SkillWarning[];
skillsSettings?: Required<SkillsSettings>; skillsSettings?: Required<SkillsSettings>;
/** Model registry for API key resolution and model discovery */ /** Model registry for API key resolution and model discovery */
modelRegistry: ModelRegistry; modelRegistry: ModelRegistry;
@ -177,6 +182,8 @@ export class AgentSession {
private _extensionRunner: ExtensionRunner | undefined = undefined; private _extensionRunner: ExtensionRunner | undefined = undefined;
private _turnIndex = 0; private _turnIndex = 0;
private _skills: Skill[];
private _skillWarnings: SkillWarning[];
private _skillsSettings: Required<SkillsSettings> | undefined; private _skillsSettings: Required<SkillsSettings> | undefined;
// Model registry for API key resolution // Model registry for API key resolution
@ -198,6 +205,8 @@ export class AgentSession {
this._scopedModels = config.scopedModels ?? []; this._scopedModels = config.scopedModels ?? [];
this._promptTemplates = config.promptTemplates ?? []; this._promptTemplates = config.promptTemplates ?? [];
this._extensionRunner = config.extensionRunner; this._extensionRunner = config.extensionRunner;
this._skills = config.skills ?? [];
this._skillWarnings = config.skillWarnings ?? [];
this._skillsSettings = config.skillsSettings; this._skillsSettings = config.skillsSettings;
this._modelRegistry = config.modelRegistry; this._modelRegistry = config.modelRegistry;
this._toolRegistry = config.toolRegistry ?? new Map(); this._toolRegistry = config.toolRegistry ?? new Map();
@ -870,6 +879,16 @@ export class AgentSession {
return this._skillsSettings; return this._skillsSettings;
} }
/** Skills loaded by SDK (empty if --no-skills or skills: [] was passed) */
get skills(): readonly Skill[] {
return this._skills;
}
/** Skill loading warnings captured by SDK */
get skillWarnings(): readonly SkillWarning[] {
return this._skillWarnings;
}
/** /**
* Abort current operation and wait for agent to become idle. * Abort current operation and wait for agent to become idle.
*/ */

View file

@ -43,7 +43,7 @@ import { ModelRegistry } from "./model-registry.js";
import { loadPromptTemplates as loadPromptTemplatesInternal, type PromptTemplate } from "./prompt-templates.js"; import { loadPromptTemplates as loadPromptTemplatesInternal, type PromptTemplate } from "./prompt-templates.js";
import { SessionManager } from "./session-manager.js"; import { SessionManager } from "./session-manager.js";
import { type Settings, SettingsManager, type SkillsSettings } from "./settings-manager.js"; import { type Settings, SettingsManager, type SkillsSettings } from "./settings-manager.js";
import { loadSkills as loadSkillsInternal, type Skill } from "./skills.js"; import { loadSkills as loadSkillsInternal, type Skill, type SkillWarning } from "./skills.js";
import { import {
buildSystemPrompt as buildSystemPromptInternal, buildSystemPrompt as buildSystemPromptInternal,
loadProjectContextFiles as loadContextFilesInternal, loadProjectContextFiles as loadContextFilesInternal,
@ -225,13 +225,16 @@ export async function discoverExtensions(
/** /**
* Discover skills from cwd and agentDir. * Discover skills from cwd and agentDir.
*/ */
export function discoverSkills(cwd?: string, agentDir?: string, settings?: SkillsSettings): Skill[] { export function discoverSkills(
const { skills } = loadSkillsInternal({ cwd?: string,
agentDir?: string,
settings?: SkillsSettings,
): { skills: Skill[]; warnings: SkillWarning[] } {
return loadSkillsInternal({
...settings, ...settings,
cwd: cwd ?? process.cwd(), cwd: cwd ?? process.cwd(),
agentDir: agentDir ?? getDefaultAgentDir(), agentDir: agentDir ?? getDefaultAgentDir(),
}); });
return skills;
} }
/** /**
@ -419,7 +422,16 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
thinkingLevel = "off"; thinkingLevel = "off";
} }
const skills = options.skills ?? discoverSkills(cwd, agentDir, settingsManager.getSkillsSettings()); let skills: Skill[];
let skillWarnings: SkillWarning[];
if (options.skills !== undefined) {
skills = options.skills;
skillWarnings = [];
} else {
const discovered = discoverSkills(cwd, agentDir, settingsManager.getSkillsSettings());
skills = discovered.skills;
skillWarnings = discovered.warnings;
}
time("discoverSkills"); time("discoverSkills");
const contextFiles = options.contextFiles ?? discoverContextFiles(cwd, agentDir); const contextFiles = options.contextFiles ?? discoverContextFiles(cwd, agentDir);
@ -641,13 +653,6 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
sessionManager.appendThinkingLevelChange(thinkingLevel); sessionManager.appendThinkingLevelChange(thinkingLevel);
} }
// Determine skillsSettings: if options.skills was explicitly provided (even []),
// mark skills as disabled so UI doesn't re-discover them
const skillsSettings =
options.skills !== undefined
? { ...settingsManager.getSkillsSettings(), enabled: false }
: settingsManager.getSkillsSettings();
const session = new AgentSession({ const session = new AgentSession({
agent, agent,
sessionManager, sessionManager,
@ -655,7 +660,9 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
scopedModels: options.scopedModels, scopedModels: options.scopedModels,
promptTemplates: promptTemplates, promptTemplates: promptTemplates,
extensionRunner, extensionRunner,
skillsSettings, skills,
skillWarnings,
skillsSettings: settingsManager.getSkillsSettings(),
modelRegistry, modelRegistry,
toolRegistry: wrappedToolRegistry ?? toolRegistry, toolRegistry: wrappedToolRegistry ?? toolRegistry,
rebuildSystemPrompt, rebuildSystemPrompt,

View file

@ -43,7 +43,6 @@ import type {
import { KeybindingsManager } from "../../core/keybindings.js"; import { KeybindingsManager } from "../../core/keybindings.js";
import { createCompactionSummaryMessage } from "../../core/messages.js"; import { createCompactionSummaryMessage } from "../../core/messages.js";
import { type SessionContext, SessionManager } from "../../core/session-manager.js"; import { type SessionContext, SessionManager } from "../../core/session-manager.js";
import { loadSkills } from "../../core/skills.js";
import { loadProjectContextFiles } from "../../core/system-prompt.js"; import { loadProjectContextFiles } from "../../core/system-prompt.js";
import { allTools } from "../../core/tools/index.js"; import { allTools } from "../../core/tools/index.js";
import type { TruncationResult } from "../../core/tools/truncate.js"; import type { TruncationResult } from "../../core/tools/truncate.js";
@ -564,24 +563,22 @@ export class InteractiveMode {
this.chatContainer.addChild(new Spacer(1)); this.chatContainer.addChild(new Spacer(1));
} }
// Show loaded skills // Show loaded skills (already discovered by SDK)
const skillsSettings = this.session.skillsSettings; const skills = this.session.skills;
if (skillsSettings?.enabled !== false) { if (skills.length > 0) {
const { skills, warnings: skillWarnings } = loadSkills(skillsSettings ?? {}); const skillList = skills.map((s) => theme.fg("dim", ` ${s.filePath}`)).join("\n");
if (skills.length > 0) { this.chatContainer.addChild(new Text(theme.fg("muted", "Loaded skills:\n") + skillList, 0, 0));
const skillList = skills.map((s) => theme.fg("dim", ` ${s.filePath}`)).join("\n"); this.chatContainer.addChild(new Spacer(1));
this.chatContainer.addChild(new Text(theme.fg("muted", "Loaded skills:\n") + skillList, 0, 0)); }
this.chatContainer.addChild(new Spacer(1));
}
// Show skill warnings if any // Show skill warnings if any
if (skillWarnings.length > 0) { const skillWarnings = this.session.skillWarnings;
const warningList = skillWarnings if (skillWarnings.length > 0) {
.map((w) => theme.fg("warning", ` ${w.skillPath}: ${w.message}`)) const warningList = skillWarnings
.join("\n"); .map((w) => theme.fg("warning", ` ${w.skillPath}: ${w.message}`))
this.chatContainer.addChild(new Text(theme.fg("warning", "Skill warnings:\n") + warningList, 0, 0)); .join("\n");
this.chatContainer.addChild(new Spacer(1)); this.chatContainer.addChild(new Text(theme.fg("warning", "Skill warnings:\n") + warningList, 0, 0));
} this.chatContainer.addChild(new Spacer(1));
} }
const extensionRunner = this.session.extensionRunner; const extensionRunner = this.session.extensionRunner;

View file

@ -14,7 +14,7 @@ describe("createAgentSession skills option", () => {
skillsDir = join(tempDir, "skills", "test-skill"); skillsDir = join(tempDir, "skills", "test-skill");
mkdirSync(skillsDir, { recursive: true }); mkdirSync(skillsDir, { recursive: true });
// Create a test skill // Create a test skill in the pi skills directory
writeFileSync( writeFileSync(
join(skillsDir, "SKILL.md"), join(skillsDir, "SKILL.md"),
`--- `---
@ -35,18 +35,19 @@ This is a test skill.
} }
}); });
it("should discover skills by default", async () => { it("should discover skills by default and expose them on session.skills", async () => {
const { session } = await createAgentSession({ const { session } = await createAgentSession({
cwd: tempDir, cwd: tempDir,
agentDir: tempDir, agentDir: tempDir,
sessionManager: SessionManager.inMemory(), sessionManager: SessionManager.inMemory(),
}); });
// skillsSettings.enabled should be true (from default settings) // Skills should be discovered and exposed on the session
expect(session.skillsSettings?.enabled).toBe(true); expect(session.skills.length).toBeGreaterThan(0);
expect(session.skills.some((s) => s.name === "test-skill")).toBe(true);
}); });
it("should disable skills in skillsSettings when options.skills is empty array", async () => { it("should have empty skills when options.skills is empty array (--no-skills)", async () => {
const { session } = await createAgentSession({ const { session } = await createAgentSession({
cwd: tempDir, cwd: tempDir,
agentDir: tempDir, agentDir: tempDir,
@ -54,27 +55,31 @@ This is a test skill.
skills: [], // Explicitly empty - like --no-skills skills: [], // Explicitly empty - like --no-skills
}); });
// skillsSettings.enabled should be false so UI doesn't re-discover // session.skills should be empty
expect(session.skillsSettings?.enabled).toBe(false); expect(session.skills).toEqual([]);
// No warnings since we didn't discover
expect(session.skillWarnings).toEqual([]);
}); });
it("should disable skills in skillsSettings when options.skills is provided with skills", async () => { it("should use provided skills when options.skills is explicitly set", async () => {
const customSkill = {
name: "custom-skill",
description: "A custom skill",
filePath: "/fake/path/SKILL.md",
baseDir: "/fake/path",
source: "custom" as const,
};
const { session } = await createAgentSession({ const { session } = await createAgentSession({
cwd: tempDir, cwd: tempDir,
agentDir: tempDir, agentDir: tempDir,
sessionManager: SessionManager.inMemory(), sessionManager: SessionManager.inMemory(),
skills: [ skills: [customSkill],
{
name: "custom-skill",
description: "A custom skill",
filePath: "/fake/path/SKILL.md",
baseDir: "/fake/path",
source: "custom",
},
],
}); });
// skillsSettings.enabled should be false because skills were explicitly provided // session.skills should contain only the provided skill
expect(session.skillsSettings?.enabled).toBe(false); expect(session.skills).toEqual([customSkill]);
// No warnings since we didn't discover
expect(session.skillWarnings).toEqual([]);
}); });
}); });