From 18ee97751ad91326d78c831036982da8f83a67c9 Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 10:45:10 +0900
Subject: [PATCH 01/14] feat(coding-agent): opt-in experimental auto-learn
(memory + isolated managed skills)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add a default-off "auto-learn" loop. When `autolearn.enabled` is set, after the
agent stops a session controller nudges it to capture reusable lessons: durable
facts go to long-term memory and repeatable procedures become "managed skills" —
SKILL.md files written to an isolated ~/.omp/agent/managed-skills directory that is
discovered and surfaced like authored skills but never overwrites them.
Two tools back this:
- `manage_skill` — create/update/delete managed skills.
- `learn` — record a lesson, optionally minting/enhancing a managed skill in the
same call (requires a hindsight/mnemopi memory backend).
The nudge is passive by default (a hidden reminder rides the next turn);
`autolearn.autoContinue` instead auto-runs one capture turn at stop, and
`autolearn.minToolCalls` (default 5) gates trivial turns. Plan/goal-mode turns and
subagents are never nudged, and the controller re-checks the live setting at fire
time so a mid-session opt-out takes effect.
Isolation & precedence: managed skills are a separate lowest-priority discovery
provider, so an authored skill of the same name wins across every provider and
custom directory regardless of third-party toggles; a disabled higher-priority
authored skill can never hide a managed one, and managed never masks an enabled
authored skill. Managed names and descriptions are sanitized on both write and
read (control/format chars, angle brackets, and Markdown fences) before they render
into the system prompt, and the SKILL.md byte cap is enforced on the final
serialized file.
Default off → zero footprint when disabled.
---
docs/settings.md | 3 +
packages/coding-agent/CHANGELOG.md | 1 +
.../coding-agent/src/autolearn/controller.ts | 103 ++++++++++
.../src/autolearn/managed-skills.ts | 128 ++++++++++++
.../src/config/settings-schema.ts | 31 ++-
.../coding-agent/src/discovery/builtin.ts | 24 ++-
.../coding-agent/src/extensibility/skills.ts | 66 ++++++
.../src/modes/components/settings-defs.ts | 7 +
.../system/autolearn-guidance-learn.md | 1 +
.../src/prompts/system/autolearn-guidance.md | 7 +
.../src/prompts/system/autolearn-nudge.md | 3 +
.../coding-agent/src/prompts/tools/learn.md | 7 +
.../src/prompts/tools/manage-skill.md | 9 +
packages/coding-agent/src/sdk.ts | 25 ++-
packages/coding-agent/src/tools/index.ts | 26 +++
packages/coding-agent/src/tools/learn.ts | 100 +++++++++
.../coding-agent/src/tools/manage-skill.ts | 66 ++++++
.../test/autolearn-controller.test.ts | 193 ++++++++++++++++++
.../test/autolearn-discovery.test.ts | 129 ++++++++++++
.../test/autolearn-managed-skills.test.ts | 136 ++++++++++++
.../test/autolearn-tools-gating.test.ts | 164 +++++++++++++++
.../test/tool-discovery/initial-tools.test.ts | 1 +
22 files changed, 1222 insertions(+), 8 deletions(-)
create mode 100644 packages/coding-agent/src/autolearn/controller.ts
create mode 100644 packages/coding-agent/src/autolearn/managed-skills.ts
create mode 100644 packages/coding-agent/src/prompts/system/autolearn-guidance-learn.md
create mode 100644 packages/coding-agent/src/prompts/system/autolearn-guidance.md
create mode 100644 packages/coding-agent/src/prompts/system/autolearn-nudge.md
create mode 100644 packages/coding-agent/src/prompts/tools/learn.md
create mode 100644 packages/coding-agent/src/prompts/tools/manage-skill.md
create mode 100644 packages/coding-agent/src/tools/learn.ts
create mode 100644 packages/coding-agent/src/tools/manage-skill.ts
create mode 100644 packages/coding-agent/test/autolearn-controller.test.ts
create mode 100644 packages/coding-agent/test/autolearn-discovery.test.ts
create mode 100644 packages/coding-agent/test/autolearn-managed-skills.test.ts
create mode 100644 packages/coding-agent/test/autolearn-tools-gating.test.ts
diff --git a/docs/settings.md b/docs/settings.md
index 4ae1dd652..b36dd5112 100644
--- a/docs/settings.md
+++ b/docs/settings.md
@@ -502,6 +502,9 @@ memory:
| `compaction.remoteEnabled` | boolean | `true` | Allow remote compaction service. |
| `compaction.autoContinue` | boolean | `true` | Continue automatically after compaction. |
| `memory.backend` | enum | `off` | `off`, `local`, `hindsight`, `mnemopi`. Each backend has its own `hindsight.*` / `mnemopi.*` / `memories.*` tuning keys. |
+| `autolearn.enabled` | boolean | `false` | Experimental: after the agent stops, nudge it to capture lessons to memory and create/enhance isolated managed skills under `~/.omp/agent/managed-skills`. Enables the `manage_skill` tool (and `learn` when a memory backend is active). |
+| `autolearn.autoContinue` | boolean | `false` | When `autolearn.enabled`, auto-run one capture turn at stop (uses extra tokens). Off = a passive reminder rides your next turn. |
+| `autolearn.minToolCalls` | number | `5` | Only nudge after a turn that used at least this many tools. |
`compaction` has additional tuning keys (idle compaction, supersede/drop heuristics) visible in `omp config list`. See [Compaction](./compaction.md) for the full strategy reference.
diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md
index a606b1695..f207171e2 100644
--- a/packages/coding-agent/CHANGELOG.md
+++ b/packages/coding-agent/CHANGELOG.md
@@ -40,6 +40,7 @@
- Changed session persistence internals to expose `writeTextAtomic(...)` on session storage writers for atomic whole-file replacements
- Changed online session-title generation to support tool-choice-less title models. Providers/models that cannot be forced to call a tool (chat-completions hosts without `tool_choice` support such as DeepSeek V4, and Claude Fable/Mythos) are now prompted to wrap the title in `...` markers instead of the `set_title` tool call; extraction is lenient, accepting a plain sentence or a truncated/unclosed tag. A `TITLE_SYSTEM.md` override is reused in this mode with the marker instruction appended.
+- Added an experimental, opt-in **auto-learn** loop (default off, `autolearn.enabled`). When enabled, after the agent stops it is nudged to capture reusable lessons: durable facts go to long-term memory and repeatable procedures become **managed skills** — `SKILL.md` files written to an isolated `~/.omp/agent/managed-skills` directory that is discovered and surfaced like authored skills but never overwrites user-authored skills (authored names always win). Two tools back this: `manage_skill` (create/update/delete managed skills) and `learn` (record a lesson, optionally minting a managed skill in the same call; requires a `hindsight`/`mnemopi` memory backend). The nudge is passive by default (a hidden reminder rides the next turn); `autolearn.autoContinue` instead auto-runs one capture turn at stop, and `autolearn.minToolCalls` (default 5) gates trivial turns. Plan/goal-mode turns and subagents are never nudged.
### Fixed
diff --git a/packages/coding-agent/src/autolearn/controller.ts b/packages/coding-agent/src/autolearn/controller.ts
new file mode 100644
index 000000000..2bdf2dfe8
--- /dev/null
+++ b/packages/coding-agent/src/autolearn/controller.ts
@@ -0,0 +1,103 @@
+/**
+ * Auto-learn session controller (experimental).
+ *
+ * Subscribes to the session event stream and, after a substantive turn,
+ * nudges the agent to capture reusable lessons. Default posture is passive
+ * (a hidden reminder rides the next real turn); with `autolearn.autoContinue`
+ * it auto-runs exactly one synthetic capture turn at stop.
+ *
+ * Installed once per top-level session (taskDepth 0). The subscription lives
+ * for the session's lifetime — `newSession` resets the session in place
+ * without re-running startup — so the controller needs no disposal.
+ */
+import { logger } from "@oh-my-pi/pi-utils";
+import type { Settings } from "../config/settings";
+import autolearnGuidance from "../prompts/system/autolearn-guidance.md" with { type: "text" };
+import autolearnGuidanceLearn from "../prompts/system/autolearn-guidance-learn.md" with { type: "text" };
+import autolearnNudge from "../prompts/system/autolearn-nudge.md" with { type: "text" };
+import type { AgentSession, AgentSessionEvent } from "../session/agent-session";
+
+const AUTOLEARN_NUDGE = autolearnNudge.trim();
+const DEFAULT_MIN_TOOL_CALLS = 5;
+
+/**
+ * Build the standing auto-learn guidance for the system prompt, or null when
+ * the feature is disabled. The `learn` addendum is included only when a memory
+ * backend is live (the `learn` tool requires one).
+ */
+export function buildAutoLearnInstructions(settings: Settings): string | null {
+ if (!settings.get("autolearn.enabled")) return null;
+ const learnEnabled = ["hindsight", "mnemopi"].includes(settings.get("memory.backend") ?? "");
+ const parts = [autolearnGuidance.trim()];
+ if (learnEnabled) parts.push(autolearnGuidanceLearn.trim());
+ return parts.join("\n\n");
+}
+
+export interface AutoLearnControllerOptions {
+ session: AgentSession;
+ settings: Settings;
+}
+
+export class AutoLearnController {
+ readonly #session: AgentSession;
+ readonly #settings: Settings;
+ #toolCalls = 0;
+ /** Swallow the agent_end produced by an auto-run capture turn so it cannot re-trigger. */
+ #suppressNext = false;
+
+ constructor(options: AutoLearnControllerOptions) {
+ this.#session = options.session;
+ this.#settings = options.settings;
+ // The listener closure captures `this`, so the session's listener array
+ // keeps the controller alive — no stored unsubscribe needed.
+ this.#session.subscribe(event => this.#onEvent(event));
+ }
+
+ #onEvent(event: AgentSessionEvent): void {
+ if (event.type === "tool_execution_end") {
+ this.#toolCalls++;
+ return;
+ }
+ if (event.type === "agent_end") {
+ this.#onAgentEnd();
+ }
+ }
+
+ #onAgentEnd(): void {
+ // Snapshot and reset every turn: the counter describes only the
+ // just-finished turn, so below-threshold, disabled, and plan-mode stops
+ // must not let tool calls accumulate into a later turn.
+ const toolCalls = this.#toolCalls;
+ this.#toolCalls = 0;
+
+ if (this.#suppressNext) {
+ this.#suppressNext = false;
+ return;
+ }
+ // Honor a live opt-out: the subscription outlives the setting, so re-check
+ // the current flag rather than trusting install-time state.
+ if (!this.#settings.get("autolearn.enabled")) return;
+ const minToolCalls = this.#settings.get("autolearn.minToolCalls") ?? DEFAULT_MIN_TOOL_CALLS;
+ if (toolCalls < minToolCalls) return;
+ // Never interrupt plan-mode review.
+ if (this.#session.getPlanModeState()?.enabled) return;
+
+ // Auto-run a capture turn only when explicitly enabled and not competing
+ // with a goal-mode continuation; otherwise ride the next turn passively.
+ const autoContinue =
+ this.#settings.get("autolearn.autoContinue") === true && !this.#session.getGoalModeState()?.enabled;
+ if (autoContinue) this.#suppressNext = true;
+
+ this.#session
+ .sendCustomMessage(
+ {
+ customType: "autolearn-nudge",
+ content: AUTOLEARN_NUDGE,
+ display: false,
+ attribution: "user",
+ },
+ { deliverAs: "nextTurn", triggerTurn: autoContinue },
+ )
+ .catch(err => logger.warn("auto-learn nudge delivery failed", { err }));
+ }
+}
diff --git a/packages/coding-agent/src/autolearn/managed-skills.ts b/packages/coding-agent/src/autolearn/managed-skills.ts
new file mode 100644
index 000000000..4f55aef28
--- /dev/null
+++ b/packages/coding-agent/src/autolearn/managed-skills.ts
@@ -0,0 +1,128 @@
+/**
+ * Managed-skills primitives for the experimental auto-learn feature.
+ *
+ * Managed skills are auto-generated/enhanced `SKILL.md` files kept in an
+ * isolated directory (`~/.omp/agent/managed-skills`) separate from
+ * user-authored skills (`~/.omp/agent/skills`). They are discovered and
+ * surfaced like normal skills, but every write here is confined to
+ * `getManagedSkillsDir()` — auto-management can never touch authored skills.
+ */
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
+import { isEnoent } from "@oh-my-pi/pi-utils";
+import { YAML } from "bun";
+import { SOURCE_PATHS } from "../discovery/helpers";
+
+/** Provider id stamped on discovered managed skills (distinguishes them from authored). */
+export const MANAGED_SKILLS_PROVIDER_ID = "omp-managed";
+
+/** Hard cap on a managed SKILL.md body to keep generated skills bounded. */
+export const MAX_MANAGED_SKILL_BYTES = 64_000;
+
+const SKILL_NAME_PATTERN = /^[a-z0-9][a-z0-9-]{0,63}$/;
+
+/** Resolve the isolated managed-skills directory (`~/.omp/agent/managed-skills`). */
+export function getManagedSkillsDir(home: string = os.homedir()): string {
+ return path.join(home, SOURCE_PATHS.native.userAgent, "managed-skills");
+}
+
+/**
+ * Validate + normalize a managed-skill name. Throws on anything outside the
+ * strict allowlist so a bad name can never escape `getManagedSkillsDir()`
+ * (blocks `..`, slashes, empty, and uppercase).
+ */
+export function sanitizeSkillName(raw: string): string {
+ const name = raw.trim().toLowerCase();
+ if (!SKILL_NAME_PATTERN.test(name)) {
+ throw new Error(
+ `Invalid skill name "${raw}". Use lowercase letters, digits, and hyphens (1-64 chars, starting with a letter or digit).`,
+ );
+ }
+ return name;
+}
+
+/**
+ * Whether `name` is a safe managed-skill name (the exact post-sanitize shape).
+ * Used to validate names read from disk at discovery time — a managed
+ * `SKILL.md` whose `frontmatter.name` was not produced by `sanitizeSkillName`
+ * (e.g. hand-placed) must not render unescaped into the system prompt.
+ */
+export function isValidManagedSkillName(name: string): boolean {
+ return SKILL_NAME_PATTERN.test(name);
+}
+
+/**
+ * Neutralize a machine-generated managed-skill description so it cannot break
+ * out of the system prompt's `` listing. Managed descriptions are
+ * generated from prior task content and persist across sessions, so this is a
+ * trust boundary: strip control/format chars, angle brackets (``
+ * / ``), and Markdown fence delimiters (backticks, `~~~`), then collapse
+ * to a single line. Applied on BOTH write and read so existing files are safe too.
+ */
+export function sanitizeManagedDescription(raw: string): string {
+ return raw
+ .replace(/[\p{Cc}\p{Cf}]/gu, " ")
+ .replace(/[<>`]/g, "")
+ .replace(/~{2,}/g, "~")
+ .replace(/\s+/g, " ")
+ .trim();
+}
+
+/**
+ * Serialize the minimal `name`/`description` frontmatter block via the repo's
+ * YAML helper (round-trips through `parseFrontmatter`).
+ */
+export function toSkillFrontmatter(name: string, description: string): string {
+ const frontmatter = YAML.stringify(
+ { name, description: sanitizeManagedDescription(description) },
+ null,
+ 2,
+ ).trimEnd();
+ return `---\n${frontmatter}\n---\n`;
+}
+
+export interface WriteManagedSkillInput {
+ action: "create" | "update";
+ name: string;
+ description: string;
+ body: string;
+}
+
+/** Create or update a managed `SKILL.md`. Returns the resolved file path. */
+export async function writeManagedSkill(input: WriteManagedSkillInput): Promise<{ path: string }> {
+ const name = sanitizeSkillName(input.name);
+ const content = `${toSkillFrontmatter(name, input.description)}\n${input.body.trim()}\n`;
+ // Cap the UTF-8 byte size of the FINAL file (body + description + frontmatter),
+ // not the UTF-16 code-unit length of the body alone.
+ const bytes = Buffer.byteLength(content, "utf8");
+ if (bytes > MAX_MANAGED_SKILL_BYTES) {
+ throw new Error(
+ `Managed skill is ${bytes} bytes; the limit is ${MAX_MANAGED_SKILL_BYTES}. Trim the body or description.`,
+ );
+ }
+ const file = path.join(getManagedSkillsDir(), name, "SKILL.md");
+ const exists = await Bun.file(file).exists();
+ if (input.action === "create" && exists) {
+ throw new Error(`Managed skill "${name}" already exists. Use action "update" to change it.`);
+ }
+ if (input.action === "update" && !exists) {
+ throw new Error(`Managed skill "${name}" does not exist. Use action "create" to add it.`);
+ }
+ await Bun.write(file, content);
+ return { path: file };
+}
+
+/** Delete a managed skill directory. Throws when it does not exist. */
+export async function deleteManagedSkill(name: string): Promise {
+ const safe = sanitizeSkillName(name);
+ const dir = path.join(getManagedSkillsDir(), safe);
+ try {
+ await fs.rm(dir, { recursive: true });
+ } catch (err) {
+ if (isEnoent(err)) {
+ throw new Error(`Managed skill "${safe}" does not exist.`);
+ }
+ throw err;
+ }
+}
diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts
index eeb26f495..3b445a4f5 100644
--- a/packages/coding-agent/src/config/settings-schema.ts
+++ b/packages/coding-agent/src/config/settings-schema.ts
@@ -109,7 +109,7 @@ export const TAB_GROUPS: Record = {
"Power (macOS)",
],
context: ["General", "Compaction", "Rules (TTSR)", "Experimental"],
- memory: ["General", "Mnemopi", "Hindsight"],
+ memory: ["General", "Auto-Learn", "Mnemopi", "Hindsight"],
files: ["Editing", "Reading", "Read Summaries", "LSP"],
shell: ["Bash", "Eval & Python"],
tools: [
@@ -1867,6 +1867,35 @@ export const SETTINGS_SCHEMA = {
},
},
+ // Auto-Learn (experimental): post-stop nudge to capture lessons to memory
+ // and mint/enhance isolated managed skills under ~/.omp/agent/managed-skills.
+ // Master flag is default-off → zero footprint; sub-flags gate behaviour.
+ "autolearn.enabled": {
+ type: "boolean",
+ default: false,
+ ui: {
+ tab: "memory",
+ group: "Auto-Learn",
+ label: "Auto-Learn (experimental)",
+ description:
+ "After the agent stops, nudge it to capture lessons to memory and create/enhance isolated managed skills",
+ },
+ },
+ "autolearn.autoContinue": {
+ type: "boolean",
+ default: false,
+ ui: {
+ tab: "memory",
+ group: "Auto-Learn",
+ label: "Auto-run capture at stop",
+ description:
+ "When on, auto-run one capture turn at stop (uses extra tokens). Off = passive reminder on your next turn.",
+ condition: "autolearnActive",
+ },
+ },
+ // Config-file-only knob (numbers without `options` are hidden from the UI).
+ "autolearn.minToolCalls": { type: "number", default: 5 },
+
// Mnemopi local SQLite memory backend.
"mnemopi.dbPath": {
type: "string",
diff --git a/packages/coding-agent/src/discovery/builtin.ts b/packages/coding-agent/src/discovery/builtin.ts
index 38c3307dd..060c5de65 100644
--- a/packages/coding-agent/src/discovery/builtin.ts
+++ b/packages/coding-agent/src/discovery/builtin.ts
@@ -6,6 +6,7 @@
import * as path from "node:path";
import { logger, parseFrontmatter, tryParseJson } from "@oh-my-pi/pi-utils";
import { YAML } from "bun";
+import { getManagedSkillsDir, MANAGED_SKILLS_PROVIDER_ID } from "../autolearn/managed-skills";
import { registerProvider } from "../capability";
import { type ContextFile, contextFileCapability } from "../capability/context-file";
import { type Extension, type ExtensionManifest, extensionCapability } from "../capability/extension";
@@ -283,13 +284,26 @@ async function loadSkills(ctx: LoadContext): Promise> {
});
const results = await Promise.all([...projectScans, userScan]);
-
return {
items: results.flatMap(r => r.items),
warnings: results.flatMap(r => r.warnings ?? []),
};
}
+// Managed skills (auto-learn) are a SEPARATE provider at the lowest skill
+// priority, so an authored skill of the same name from ANY other provider wins
+// the capability-level priority dedup. Discovery is unconditional (an empty
+// managed dir is a no-op); only writing/nudging is gated by `autolearn.enabled`.
+const MANAGED_SKILLS_PRIORITY = 5;
+async function loadManagedSkills(ctx: LoadContext): Promise> {
+ return scanSkillsFromDir(ctx, {
+ dir: getManagedSkillsDir(ctx.home),
+ providerId: MANAGED_SKILLS_PROVIDER_ID,
+ level: "user",
+ requireDescription: true,
+ });
+}
+
registerProvider(skillCapability.id, {
id: PROVIDER_ID,
displayName: DISPLAY_NAME,
@@ -298,6 +312,14 @@ registerProvider(skillCapability.id, {
load: loadSkills,
});
+registerProvider(skillCapability.id, {
+ id: MANAGED_SKILLS_PROVIDER_ID,
+ displayName: "Managed Skills (auto-learn)",
+ description: "Auto-generated managed skills from ~/.omp/agent/managed-skills",
+ priority: MANAGED_SKILLS_PRIORITY,
+ load: loadManagedSkills,
+});
+
// Slash Commands
async function loadSlashCommands(ctx: LoadContext): Promise> {
const items: SlashCommand[] = [];
diff --git a/packages/coding-agent/src/extensibility/skills.ts b/packages/coding-agent/src/extensibility/skills.ts
index 265ceaf92..5724e469e 100644
--- a/packages/coding-agent/src/extensibility/skills.ts
+++ b/packages/coding-agent/src/extensibility/skills.ts
@@ -1,6 +1,11 @@
import * as fs from "node:fs/promises";
import * as os from "node:os";
import { getProjectDir } from "@oh-my-pi/pi-utils";
+import {
+ isValidManagedSkillName,
+ MANAGED_SKILLS_PROVIDER_ID,
+ sanitizeManagedDescription,
+} from "../autolearn/managed-skills";
import { skillCapability } from "../capability/skill";
import type { SourceMeta } from "../capability/types";
import type { SkillsSettings } from "../config/settings";
@@ -137,6 +142,10 @@ export async function loadSkills(options: LoadSkillsOptions = {}): Promise
+ capSkill._source.provider === MANAGED_SKILLS_PROVIDER_ID &&
+ isValidManagedSkillName(capSkill.name) &&
+ !disabledSkillNames.has(capSkill.name) &&
+ !matchesIgnorePatterns(capSkill.name) &&
+ matchesIncludePatterns(capSkill.name),
+ );
+ // Names claimed by any ENABLED authored skill (from the pre-dedup superset).
+ // Managed defers to these even when capability dedup hid an enabled authored
+ // skill behind a disabled higher-priority one, so managed never masks it.
+ const enabledAuthoredNames = new Set(
+ result.all
+ .filter(
+ capSkill => capSkill._source.provider !== MANAGED_SKILLS_PROVIDER_ID && isSourceEnabled(capSkill._source),
+ )
+ .map(capSkill => capSkill.name),
+ );
+ const managedRealPaths = await Promise.all(
+ managedCandidates.map(async capSkill => {
+ try {
+ return await fs.realpath(capSkill.path);
+ } catch {
+ return capSkill.path;
+ }
+ }),
+ );
+ for (let i = 0; i < managedCandidates.length; i++) {
+ const capSkill = managedCandidates[i];
+ const resolvedPath = managedRealPaths[i];
+ if (realPathSet.has(resolvedPath)) continue;
+ if (enabledAuthoredNames.has(capSkill.name)) continue; // an enabled authored skill owns this name
+ if (skillMap.has(capSkill.name)) continue; // already claimed (e.g. custom dir)
+ const rawDescription =
+ typeof capSkill.frontmatter?.description === "string" ? capSkill.frontmatter.description : "";
+ skillMap.set(capSkill.name, {
+ name: capSkill.name,
+ description: sanitizeManagedDescription(rawDescription),
+ filePath: capSkill.path,
+ baseDir: capSkill.path.replace(/[\\/]SKILL\.md$/, ""),
+ source: `${capSkill._source.provider}:${capSkill.level}`,
+ hide: capSkill.frontmatter?.hide === true || capSkill.frontmatter?.disableModelInvocation === true,
+ _source: capSkill._source,
+ });
+ realPathSet.add(resolvedPath);
+ }
+
const skills = Array.from(skillMap.values());
// Deterministic ordering for prompt stability (case-insensitive, then exact name, then path).
skills.sort((a, b) => compareSkillOrder(a.name, a.filePath, b.name, b.filePath));
diff --git a/packages/coding-agent/src/modes/components/settings-defs.ts b/packages/coding-agent/src/modes/components/settings-defs.ts
index 1d04f3596..8c47c8c5e 100644
--- a/packages/coding-agent/src/modes/components/settings-defs.ts
+++ b/packages/coding-agent/src/modes/components/settings-defs.ts
@@ -90,6 +90,13 @@ const CONDITIONS: Record boolean> = {
return false;
}
},
+ autolearnActive: () => {
+ try {
+ return Settings.instance.get("autolearn.enabled") === true;
+ } catch {
+ return false;
+ }
+ },
autoThinkingActive: () => {
try {
return Settings.instance.get("defaultThinkingLevel") === "auto";
diff --git a/packages/coding-agent/src/prompts/system/autolearn-guidance-learn.md b/packages/coding-agent/src/prompts/system/autolearn-guidance-learn.md
new file mode 100644
index 000000000..68211eb3a
--- /dev/null
+++ b/packages/coding-agent/src/prompts/system/autolearn-guidance-learn.md
@@ -0,0 +1 @@
+When a lesson is a durable *fact* rather than a procedure — a project convention, a non-obvious fix, a user preference — record it with `learn`, which writes to long-term memory. `learn` can also mint or enhance a managed skill in the same call when the lesson is both a fact and a procedure.
diff --git a/packages/coding-agent/src/prompts/system/autolearn-guidance.md b/packages/coding-agent/src/prompts/system/autolearn-guidance.md
new file mode 100644
index 000000000..509ce1f66
--- /dev/null
+++ b/packages/coding-agent/src/prompts/system/autolearn-guidance.md
@@ -0,0 +1,7 @@
+## Auto-Learn (experimental)
+
+You can grow a library of reusable **managed skills** with the `manage_skill` tool. Managed skills are `SKILL.md` files kept in an isolated directory (`~/.omp/agent/managed-skills`); they are surfaced to you in future sessions like any other skill.
+
+- Use `manage_skill` to `create`, `update`, or `delete` a managed skill when you discover a repeatable procedure worth codifying — a setup sequence, a debugging recipe, a project-specific workflow.
+- **Isolation rule:** managed skills are the ONLY skills you may write. NEVER edit user-authored skills under `~/.omp/agent/skills` or `.omp/skills`.
+- Capture sparingly and specifically. A skill earns its place only if it will be reused; prefer enhancing an existing managed skill over creating a near-duplicate.
diff --git a/packages/coding-agent/src/prompts/system/autolearn-nudge.md b/packages/coding-agent/src/prompts/system/autolearn-nudge.md
new file mode 100644
index 000000000..2ee8322ba
--- /dev/null
+++ b/packages/coding-agent/src/prompts/system/autolearn-nudge.md
@@ -0,0 +1,3 @@
+Before you finish: if this turn produced anything reusable, capture it now with your learning tools — a repeatable procedure becomes a managed skill (`manage_skill`), and a durable fact or convention is worth remembering (`learn`, when memory is enabled).
+
+Only capture what will genuinely help next time. If nothing this turn is worth keeping, do nothing.
diff --git a/packages/coding-agent/src/prompts/tools/learn.md b/packages/coding-agent/src/prompts/tools/learn.md
new file mode 100644
index 000000000..ed412ad5b
--- /dev/null
+++ b/packages/coding-agent/src/prompts/tools/learn.md
@@ -0,0 +1,7 @@
+Capture a reusable lesson into long-term memory, and optionally mint or enhance a managed skill in the same call.
+
+Use after solving something whose insight will pay off again: a non-obvious fix, a project convention you had to discover, a workflow that worked. The `memory` field is the durable, self-contained lesson — include what, when, and why so a future session understands it without this conversation.
+
+Provide the optional `skill` object when the lesson is a repeatable *procedure* worth codifying as a `SKILL.md` (not just a fact). Managed skills are written to an isolated directory (`~/.omp/agent/managed-skills`) and are surfaced like normal skills next session. They NEVER touch user-authored skills. `body` is the SKILL.md content in markdown — do not include frontmatter; it is generated from `name` and `description`. Use `action: "update"` to enhance an existing managed skill.
+
+Capture sparingly and specifically. One strong, reusable lesson beats several vague ones.
diff --git a/packages/coding-agent/src/prompts/tools/manage-skill.md b/packages/coding-agent/src/prompts/tools/manage-skill.md
new file mode 100644
index 000000000..d957964f0
--- /dev/null
+++ b/packages/coding-agent/src/prompts/tools/manage-skill.md
@@ -0,0 +1,9 @@
+Create, update, or delete a managed skill — a `SKILL.md` written to an isolated directory (`~/.omp/agent/managed-skills`) and surfaced like a normal skill in future sessions.
+
+Managed skills are for repeatable procedures worth codifying: a setup sequence, a debugging recipe, a project-specific workflow. They are kept separate from user-authored skills and this tool NEVER edits those.
+
+- `action: "create"` — requires `name`, `description`, and `body`. Fails if the skill already exists.
+- `action: "update"` — requires `name`, `description`, and `body`. Fails if the skill does not exist. Overwrites the body.
+- `action: "delete"` — requires `name`. Fails if the skill does not exist.
+
+`name` is kebab-case (lowercase letters, digits, hyphens). `description` is a single line stating when to use the skill — it drives discovery, so make it specific. `body` is the SKILL.md content in markdown; do not include frontmatter (it is generated from `name` and `description`).
diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts
index faf977da2..915f6562e 100644
--- a/packages/coding-agent/src/sdk.ts
+++ b/packages/coding-agent/src/sdk.ts
@@ -36,6 +36,7 @@ import {
} from "@oh-my-pi/pi-utils";
import chalk from "chalk";
import { type AsyncJob, AsyncJobManager } from "./async";
+import { AutoLearnController, buildAutoLearnInstructions } from "./autolearn/controller";
import { loadCapability } from "./capability";
import { type Rule, ruleCapability, setActiveRules } from "./capability/rule";
import { bucketRules } from "./capability/rule-buckets";
@@ -2113,13 +2114,18 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
const memoryBackend = await resolveMemoryBackend(settings);
const memoryInstructions = await memoryBackend.buildDeveloperInstructions(agentDir, settings, session);
- // Build combined append prompt: memory instructions + MCP server instructions.
- // For UI sessions MCP discovery is deferred, so `getServerInstructions()` is
- // empty until the background connect completes; the rebuild that
- // `refreshMCPTools` triggers post-discovery then picks up the now-connected
- // servers' instructions, so they join the prompt for the rest of the session.
+ // Build combined append prompt: memory instructions + auto-learn guidance
+ // + MCP server instructions. For UI sessions MCP discovery is deferred, so
+ // `getServerInstructions()` is empty until the background connect completes;
+ // the rebuild that `refreshMCPTools` triggers post-discovery then picks up
+ // the now-connected servers' instructions, so they join the prompt for the
+ // rest of the session.
const serverInstructions = mcpManager?.getServerInstructions();
- let appendPrompt: string | undefined = memoryInstructions ?? undefined;
+ const autoLearnInstructions = buildAutoLearnInstructions(settings);
+ const appendParts: string[] = [];
+ if (memoryInstructions) appendParts.push(memoryInstructions);
+ if (autoLearnInstructions) appendParts.push(autoLearnInstructions);
+ let appendPrompt: string | undefined = appendParts.length > 0 ? appendParts.join("\n\n") : undefined;
if (serverInstructions && serverInstructions.size > 0) {
const parts: string[] = [];
if (appendPrompt) parts.push(appendPrompt);
@@ -2663,6 +2669,13 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
parentHindsightSessionState: options.parentHindsightSessionState,
parentMnemopiSessionState: options.parentMnemopiSessionState,
});
+
+ // Install the auto-learn post-stop nudge controller once, for top-level
+ // sessions only. The subscription lives for the session's lifetime; the
+ // reference is intentionally discarded (the listener retains it).
+ if (settings.get("autolearn.enabled") && taskDepth === 0) {
+ new AutoLearnController({ session, settings });
+ }
});
// Wire MCP manager callbacks to session for reactive tool updates.
diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts
index 82099eeda..22fb54586 100644
--- a/packages/coding-agent/src/tools/index.ts
+++ b/packages/coding-agent/src/tools/index.ts
@@ -46,6 +46,8 @@ import { GithubTool } from "./gh";
import { InspectImageTool } from "./inspect-image";
import { IrcTool, isIrcEnabled } from "./irc";
import { JobTool } from "./job";
+import { LearnTool } from "./learn";
+import { ManageSkillTool } from "./manage-skill";
import { MemoryEditTool } from "./memory-edit";
import { MemoryRecallTool } from "./memory-recall";
import { MemoryReflectTool } from "./memory-reflect";
@@ -84,6 +86,8 @@ export * from "./image-gen";
export * from "./inspect-image";
export * from "./irc";
export * from "./job";
+export * from "./learn";
+export * from "./manage-skill";
export * from "./memory-edit";
export * from "./memory-recall";
export * from "./memory-reflect";
@@ -439,6 +443,8 @@ export const BUILTIN_TOOLS: Record = {
retain: MemoryRetainTool.createIf,
recall: MemoryRecallTool.createIf,
reflect: MemoryReflectTool.createIf,
+ learn: LearnTool.createIf,
+ manage_skill: ManageSkillTool.createIf,
};
export const HIDDEN_TOOLS: Record = {
@@ -519,6 +525,19 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P
if (!requestedTools.includes(name)) requestedTools.push(name);
}
}
+ // Auto-learn tools are gated by `autolearn.enabled` but, like the memory
+ // tools above, must also be force-included into an explicit requestedTools
+ // list so a restricted session whose controller/guidance is active still
+ // exposes the tools the nudge points at.
+ if (session.settings.get("autolearn.enabled")) {
+ if (!requestedTools.includes("manage_skill")) requestedTools.push("manage_skill");
+ if (
+ ["hindsight", "mnemopi"].includes(session.settings.get("memory.backend") ?? "") &&
+ !requestedTools.includes("learn")
+ ) {
+ requestedTools.push("learn");
+ }
+ }
}
// Resolve effective tool discovery mode.
// tools.discoveryMode controls the new modes; mcp.discoveryMode remains a back-compat alias for "mcp-only".
@@ -552,6 +571,13 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P
if (name === "retain" || name === "recall" || name === "reflect") {
return ["hindsight", "mnemopi"].includes(session.settings.get("memory.backend") ?? "");
}
+ if (name === "manage_skill") return session.settings.get("autolearn.enabled");
+ if (name === "learn") {
+ return (
+ session.settings.get("autolearn.enabled") &&
+ ["hindsight", "mnemopi"].includes(session.settings.get("memory.backend") ?? "")
+ );
+ }
if (name === "task") {
return canSpawnAtDepth(session.settings.get("task.maxRecursionDepth") ?? 2, session.taskDepth ?? 0);
}
diff --git a/packages/coding-agent/src/tools/learn.ts b/packages/coding-agent/src/tools/learn.ts
new file mode 100644
index 000000000..9dea74df8
--- /dev/null
+++ b/packages/coding-agent/src/tools/learn.ts
@@ -0,0 +1,100 @@
+import type { AgentTool, AgentToolResult } from "@oh-my-pi/pi-agent-core";
+import { z } from "zod/v4";
+import { writeManagedSkill } from "../autolearn/managed-skills";
+import learnDescription from "../prompts/tools/learn.md" with { type: "text" };
+import type { ToolSession } from ".";
+
+const learnSchema = z.object({
+ memory: z.string().describe("the durable, self-contained lesson to remember (what, when, why)"),
+ context: z.string().describe("optional source context for the lesson").optional(),
+ skill: z
+ .object({
+ action: z.enum(["create", "update"]),
+ name: z.string().describe("kebab-case skill name"),
+ description: z.string().describe("one-line description of when to use the skill"),
+ body: z.string().describe("the SKILL.md body in markdown (no frontmatter)"),
+ })
+ .describe("also create or enhance a managed skill in the same call")
+ .optional(),
+});
+
+export type LearnParams = z.infer;
+
+/**
+ * Orchestrating "learn" tool: persists a lesson to long-term memory and,
+ * given a `skill` payload, mints/enhances a managed skill via the shared
+ * `writeManagedSkill` primitive. Gated behind `autolearn.enabled` plus a live
+ * memory backend (the memory half needs one).
+ */
+export class LearnTool implements AgentTool {
+ readonly name = "learn";
+ readonly approval = (args: unknown) => ((args as Partial).skill ? "write" : "read");
+ readonly label = "Learn";
+ readonly description = learnDescription;
+ readonly parameters = learnSchema;
+ readonly strict = true;
+ readonly loadMode = "essential" as const;
+ readonly summary = "Capture a reusable lesson to memory (and optionally a managed skill)";
+
+ constructor(private readonly session: ToolSession) {}
+
+ static createIf(session: ToolSession): LearnTool | null {
+ if (!session.settings.get("autolearn.enabled")) return null;
+ const backend = session.settings.get("memory.backend");
+ if (backend !== "hindsight" && backend !== "mnemopi") return null;
+ return new LearnTool(session);
+ }
+
+ async execute(_id: string, params: LearnParams): Promise {
+ // 1) Persist the lesson to long-term memory (mirrors MemoryRetainTool).
+ const backend = this.session.settings.get("memory.backend");
+ if (backend === "mnemopi") {
+ const state = this.session.getMnemopiSessionState?.();
+ if (!state) {
+ throw new Error("Mnemopi backend is not initialised for this session.");
+ }
+ state.rememberScoped(params.memory, {
+ source: "coding-agent-learn",
+ importance: 0.8,
+ metadata: {
+ session_id: state.sessionId,
+ cwd: state.session.sessionManager.getCwd(),
+ context: params.context ?? null,
+ tool: "learn",
+ },
+ scope: "bank",
+ extract: true,
+ extractEntities: true,
+ veracity: "tool",
+ memoryType: "fact",
+ });
+ } else {
+ const state = this.session.getHindsightSessionState?.();
+ if (!state) {
+ throw new Error("Hindsight backend is not initialised for this session.");
+ }
+ state.enqueueRetain(params.memory, params.context);
+ }
+
+ // 2) Optionally mint/enhance a managed skill. A failure here is surfaced
+ // as a partial outcome — the lesson is already persisted.
+ if (params.skill) {
+ try {
+ await writeManagedSkill(params.skill);
+ } catch (err) {
+ const reason = err instanceof Error ? err.message : String(err);
+ throw new Error(`Lesson stored, but the managed skill could not be written: ${reason}`);
+ }
+ const verb = params.skill.action === "create" ? "Created" : "Updated";
+ return {
+ content: [{ type: "text", text: `Lesson stored. ${verb} managed skill "${params.skill.name}".` }],
+ details: { skill: params.skill.name },
+ };
+ }
+
+ return {
+ content: [{ type: "text", text: "Lesson stored." }],
+ details: { skill: null },
+ };
+ }
+}
diff --git a/packages/coding-agent/src/tools/manage-skill.ts b/packages/coding-agent/src/tools/manage-skill.ts
new file mode 100644
index 000000000..f82fa802a
--- /dev/null
+++ b/packages/coding-agent/src/tools/manage-skill.ts
@@ -0,0 +1,66 @@
+import * as path from "node:path";
+import type { AgentTool, AgentToolResult } from "@oh-my-pi/pi-agent-core";
+import { z } from "zod/v4";
+import { deleteManagedSkill, getManagedSkillsDir, writeManagedSkill } from "../autolearn/managed-skills";
+import manageSkillDescription from "../prompts/tools/manage-skill.md" with { type: "text" };
+import type { ToolSession } from ".";
+
+const manageSkillSchema = z.object({
+ action: z.enum(["create", "update", "delete"]),
+ name: z.string().describe("kebab-case skill name"),
+ description: z
+ .string()
+ .describe("one-line description of when to use the skill (required for create/update)")
+ .optional(),
+ body: z.string().describe("the SKILL.md body in markdown, no frontmatter (required for create/update)").optional(),
+});
+
+export type ManageSkillParams = z.infer;
+
+/**
+ * Direct create/update/delete of isolated managed skills. Gated behind
+ * `autolearn.enabled`; backend-independent (the skill side is standalone).
+ */
+export class ManageSkillTool implements AgentTool {
+ readonly name = "manage_skill";
+ readonly approval = "write" as const;
+ readonly label = "Manage Skill";
+ readonly description = manageSkillDescription;
+ readonly parameters = manageSkillSchema;
+ readonly strict = true;
+ readonly loadMode = "essential" as const;
+ readonly summary = "Create, update, or delete an isolated managed skill";
+
+ // No session state needed: createIf reads settings; writes target the
+ // home-based managed-skills dir directly.
+ static createIf(session: ToolSession): ManageSkillTool | null {
+ if (!session.settings.get("autolearn.enabled")) return null;
+ return new ManageSkillTool();
+ }
+
+ async execute(_id: string, params: ManageSkillParams): Promise {
+ if (params.action === "delete") {
+ await deleteManagedSkill(params.name);
+ return {
+ content: [{ type: "text", text: `Deleted managed skill "${params.name}".` }],
+ details: { action: "delete", name: params.name },
+ };
+ }
+
+ if (!params.description || !params.body) {
+ throw new Error(`"${params.action}" requires both "description" and "body".`);
+ }
+ const { path: skillPath } = await writeManagedSkill({
+ action: params.action,
+ name: params.name,
+ description: params.description,
+ body: params.body,
+ });
+ const relativePath = path.relative(getManagedSkillsDir(), skillPath);
+ const verb = params.action === "create" ? "Created" : "Updated";
+ return {
+ content: [{ type: "text", text: `${verb} managed skill "${params.name}" (managed-skills/${relativePath}).` }],
+ details: { action: params.action, name: params.name },
+ };
+ }
+}
diff --git a/packages/coding-agent/test/autolearn-controller.test.ts b/packages/coding-agent/test/autolearn-controller.test.ts
new file mode 100644
index 000000000..9b3dc2e83
--- /dev/null
+++ b/packages/coding-agent/test/autolearn-controller.test.ts
@@ -0,0 +1,193 @@
+import { describe, expect, it } from "bun:test";
+import { AutoLearnController, buildAutoLearnInstructions } from "@oh-my-pi/pi-coding-agent/autolearn/controller";
+import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
+import type { AgentSession, AgentSessionEvent } from "@oh-my-pi/pi-coding-agent/session/agent-session";
+
+interface CapturedNudge {
+ message: { customType: string; content: string; display?: boolean; attribution?: string };
+ options?: { deliverAs?: string; triggerTurn?: boolean };
+}
+
+class FakeSession {
+ readonly listeners: Array<(event: AgentSessionEvent) => void> = [];
+ readonly sent: CapturedNudge[] = [];
+ planEnabled = false;
+ goalEnabled = false;
+
+ subscribe(listener: (event: AgentSessionEvent) => void): () => void {
+ this.listeners.push(listener);
+ return () => {};
+ }
+
+ async sendCustomMessage(message: CapturedNudge["message"], options?: CapturedNudge["options"]): Promise {
+ this.sent.push({ message, options });
+ }
+
+ getPlanModeState(): { enabled: boolean } | undefined {
+ return this.planEnabled ? { enabled: true } : undefined;
+ }
+
+ getGoalModeState(): { enabled: boolean } | undefined {
+ return this.goalEnabled ? { enabled: true } : undefined;
+ }
+
+ emit(event: AgentSessionEvent): void {
+ for (const listener of [...this.listeners]) listener(event);
+ }
+
+ toolCalls(n: number): void {
+ for (let i = 0; i < n; i++) {
+ this.emit({ type: "tool_execution_end", toolCallId: `t${i}`, toolName: "read", result: null });
+ }
+ }
+
+ agentEnd(): void {
+ this.emit({ type: "agent_end", messages: [] });
+ }
+}
+
+function install(session: FakeSession, overrides: Record = {}): Settings {
+ const settings = Settings.isolated({ "autolearn.enabled": true, ...overrides });
+ new AutoLearnController({ session: session as unknown as AgentSession, settings });
+ return settings;
+}
+
+describe("AutoLearnController", () => {
+ it("fires one passive nudge once the tool-call threshold is met", () => {
+ const session = new FakeSession();
+ install(session);
+ session.toolCalls(5);
+ session.agentEnd();
+
+ expect(session.sent).toHaveLength(1);
+ expect(session.sent[0]?.message.customType).toBe("autolearn-nudge");
+ expect(session.sent[0]?.message.display).toBe(false);
+ expect(session.sent[0]?.options?.deliverAs).toBe("nextTurn");
+ expect(session.sent[0]?.options?.triggerTurn).toBe(false);
+ });
+
+ it("does not nudge below the threshold", () => {
+ const session = new FakeSession();
+ install(session);
+ session.toolCalls(4);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(0);
+ });
+
+ it("does not nudge during plan mode", () => {
+ const session = new FakeSession();
+ session.planEnabled = true;
+ install(session);
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(0);
+ });
+ it("does not combine tool calls across separate sub-threshold turns", () => {
+ const session = new FakeSession();
+ install(session);
+ session.toolCalls(3);
+ session.agentEnd();
+ session.toolCalls(3);
+ session.agentEnd();
+ // Neither turn reached the threshold; the counter must not accumulate.
+ expect(session.sent).toHaveLength(0);
+ });
+
+ it("discards plan-mode tool calls instead of leaking them into the next turn", () => {
+ const session = new FakeSession();
+ session.planEnabled = true;
+ install(session);
+ session.toolCalls(5);
+ session.agentEnd(); // plan mode: no fire, counter reset
+ session.planEnabled = false;
+ session.toolCalls(1);
+ session.agentEnd(); // 1 < threshold -> no fire (no plan-mode leak)
+ expect(session.sent).toHaveLength(0);
+ });
+
+ it("stops nudging when autolearn is disabled mid-session", () => {
+ const session = new FakeSession();
+ // Enable via the global layer (not an isolated override) so the live flag
+ // can be flipped and the controller's fire-time re-check is exercised.
+ const settings = Settings.isolated({});
+ settings.set("autolearn.enabled", true);
+ new AutoLearnController({ session: session as unknown as AgentSession, settings });
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(1); // fires while enabled
+ settings.set("autolearn.enabled", false);
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(1); // no new nudge after disable
+ // The disabled stop must NOT leave its tool calls queued: re-enabling and
+ // doing a sub-threshold turn must not fire from leaked counts.
+ settings.set("autolearn.enabled", true);
+ session.toolCalls(1);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(1);
+ });
+
+ it("downgrades autoContinue to a passive nudge during goal mode", () => {
+ const session = new FakeSession();
+ session.goalEnabled = true;
+ install(session, { "autolearn.autoContinue": true });
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(1);
+ expect(session.sent[0]?.options?.triggerTurn).toBe(false);
+ // Passive => no suppression; the next qualifying stop fires again.
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(2);
+ });
+
+ it("auto-runs a capture turn and suppresses exactly one follow-up agent_end", () => {
+ const session = new FakeSession();
+ install(session, { "autolearn.autoContinue": true });
+
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(1);
+ expect(session.sent[0]?.options?.triggerTurn).toBe(true);
+
+ // The synthetic capture turn's agent_end is swallowed.
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(1);
+
+ // Suppression is one-shot: the next qualifying stop fires again.
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(2);
+ });
+
+ it("respects a custom minToolCalls threshold", () => {
+ const session = new FakeSession();
+ install(session, { "autolearn.minToolCalls": 2 });
+ session.toolCalls(2);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(1);
+ });
+});
+
+describe("buildAutoLearnInstructions", () => {
+ it("returns null when auto-learn is disabled", () => {
+ expect(buildAutoLearnInstructions(Settings.isolated({ "autolearn.enabled": false }))).toBeNull();
+ });
+
+ it("includes the learn addendum when a memory backend is live", () => {
+ const text = buildAutoLearnInstructions(
+ Settings.isolated({ "autolearn.enabled": true, "memory.backend": "mnemopi" }),
+ );
+ expect(text).toContain("manage_skill");
+ expect(text).toContain("long-term memory");
+ });
+
+ it("omits the learn addendum when no memory backend is configured", () => {
+ const text = buildAutoLearnInstructions(
+ Settings.isolated({ "autolearn.enabled": true, "memory.backend": "off" }),
+ );
+ expect(text).toContain("manage_skill");
+ expect(text).not.toContain("long-term memory");
+ });
+});
diff --git a/packages/coding-agent/test/autolearn-discovery.test.ts b/packages/coding-agent/test/autolearn-discovery.test.ts
new file mode 100644
index 000000000..caf6ce952
--- /dev/null
+++ b/packages/coding-agent/test/autolearn-discovery.test.ts
@@ -0,0 +1,129 @@
+import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test";
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
+import { getManagedSkillsDir } from "@oh-my-pi/pi-coding-agent/autolearn/managed-skills";
+import "@oh-my-pi/pi-coding-agent/discovery";
+import { loadSkills } from "@oh-my-pi/pi-coding-agent/extensibility/skills";
+
+async function writeSkill(dir: string, name: string, description: string): Promise {
+ const file = path.join(dir, name, "SKILL.md");
+ await fs.mkdir(path.dirname(file), { recursive: true });
+ await fs.writeFile(file, ["---", `description: ${description}`, "---", "", `# ${name}`].join("\n"));
+}
+
+describe("managed-skills discovery", () => {
+ let tempHome: string;
+ let tempCwd: string;
+ let managedDir: string;
+ let authoredDir: string;
+
+ beforeEach(async () => {
+ tempHome = await fs.mkdtemp(path.join(os.tmpdir(), "omp-managed-disco-home-"));
+ // cwd MUST live under the fake home so loadSkills' ancestor walk is bounded
+ // and cannot pick up ambient /tmp/.omp or /.omp fixtures (full-suite-safe).
+ tempCwd = path.join(tempHome, "work");
+ await fs.mkdir(tempCwd, { recursive: true });
+ spyOn(os, "homedir").mockReturnValue(tempHome);
+ managedDir = getManagedSkillsDir();
+ // Authored user skills live in the sibling `skills/` dir under .../agent.
+ authoredDir = path.join(path.dirname(managedDir), "skills");
+ });
+
+ afterEach(async () => {
+ spyOn(os, "homedir").mockRestore();
+ await fs.rm(tempHome, { recursive: true, force: true });
+ });
+
+ it("surfaces a managed skill tagged with the omp-managed provider", async () => {
+ await writeSkill(managedDir, "foo", "A managed skill.");
+ const { skills } = await loadSkills({ cwd: tempCwd });
+ const foo = skills.find(s => s.name === "foo");
+ expect(foo).toBeDefined();
+ expect(foo?.source).toBe("omp-managed:user");
+ });
+
+ it("lets an authored skill win a name collision and drops the managed one", async () => {
+ await writeSkill(authoredDir, "bar", "Authored bar.");
+ await writeSkill(managedDir, "bar", "Managed bar.");
+ const { skills } = await loadSkills({ cwd: tempCwd });
+ const bars = skills.filter(s => s.name === "bar");
+ expect(bars).toHaveLength(1);
+ expect(bars[0]?.source).toBe("native:user");
+ expect(skills.some(s => s.name === "bar" && s.source === "omp-managed:user")).toBe(false);
+ });
+
+ it("lets an authored skill from a NON-native provider win over a managed skill", async () => {
+ // `.agents/skills` is the `agents` provider — a different provider than the
+ // one that discovers managed skills. Authored must still win globally.
+ await writeSkill(path.join(tempHome, ".agents", "skills"), "baz", "Authored baz (.agents).");
+ await writeSkill(managedDir, "baz", "Managed baz.");
+ const { skills } = await loadSkills({ cwd: tempCwd });
+ const bazzes = skills.filter(s => s.name === "baz");
+ expect(bazzes).toHaveLength(1);
+ expect(bazzes[0]?.source).toBe("agents:user");
+ expect(skills.some(s => s.name === "baz" && s.source === "omp-managed:user")).toBe(false);
+ });
+
+ it("lets a custom-directory authored skill win over a managed skill", async () => {
+ // Custom directories are merged AFTER loadCapability, so this exercises the
+ // skills.ts dead-last backstop rather than capability-level priority dedup.
+ const customDir = path.join(tempHome, "custom-skills");
+ await writeSkill(customDir, "qux", "Authored qux (custom).");
+ await writeSkill(managedDir, "qux", "Managed qux.");
+ const { skills } = await loadSkills({ cwd: tempCwd, customDirectories: [customDir] });
+ const quxes = skills.filter(s => s.name === "qux");
+ expect(quxes).toHaveLength(1);
+ expect(quxes[0]?.source).toBe("custom:user");
+ });
+
+ it("keeps a managed skill visible even when a disabled provider has the same name", async () => {
+ // loadCapability dedupes before source filtering, so a fully-DISABLED higher-
+ // priority authored skill must not consume the managed fallback. claude is
+ // discovered at user AND (because cwd is under home) project level, so both
+ // toggles must be off to truly disable it.
+ await writeSkill(path.join(tempHome, ".claude", "skills"), "dis", "Disabled claude dis.");
+ await writeSkill(managedDir, "dis", "Managed dis.");
+ const { skills } = await loadSkills({
+ cwd: tempCwd,
+ enableClaudeUser: false,
+ enableClaudeProject: false,
+ });
+ const dises = skills.filter(s => s.name === "dis");
+ expect(dises).toHaveLength(1);
+ expect(dises[0]?.source).toBe("omp-managed:user");
+ });
+
+ it("defers a managed skill to an ENABLED authored skill hidden behind a disabled higher-priority one", async () => {
+ // claude (priority 80, fully disabled) shadows agents (70, enabled) at
+ // capability dedup; agents survives only in result.all. Managed must NOT mask
+ // the enabled authored name, so no omp-managed skill is surfaced here.
+ await writeSkill(path.join(tempHome, ".claude", "skills"), "shadowed", "Disabled claude.");
+ await writeSkill(path.join(tempHome, ".agents", "skills"), "shadowed", "Enabled agents.");
+ await writeSkill(managedDir, "shadowed", "Managed shadowed.");
+ const { skills } = await loadSkills({
+ cwd: tempCwd,
+ enableClaudeUser: false,
+ enableClaudeProject: false,
+ });
+ expect(skills.some(s => s.name === "shadowed" && s.source === "omp-managed:user")).toBe(false);
+ });
+
+ it("skips a managed skill whose on-disk frontmatter name is unsafe", async () => {
+ const dir = path.join(managedDir, "evil-holder");
+ await fs.mkdir(dir, { recursive: true });
+ await fs.writeFile(
+ path.join(dir, "SKILL.md"),
+ ["---", 'name: "evil"', "description: Evil.", "---", "", "# evil"].join("\n"),
+ );
+ const { skills } = await loadSkills({ cwd: tempCwd });
+ expect(skills.some(s => s.name.includes("<"))).toBe(false);
+ expect(skills.some(s => s.source === "omp-managed:user")).toBe(false);
+ });
+
+ it("is a no-op when the managed dir is absent", async () => {
+ const { skills, warnings } = await loadSkills({ cwd: tempCwd });
+ expect(skills.some(s => s.source === "omp-managed:user")).toBe(false);
+ expect(warnings.some(w => w.message.includes("managed-skills"))).toBe(false);
+ });
+});
diff --git a/packages/coding-agent/test/autolearn-managed-skills.test.ts b/packages/coding-agent/test/autolearn-managed-skills.test.ts
new file mode 100644
index 000000000..21f29d17e
--- /dev/null
+++ b/packages/coding-agent/test/autolearn-managed-skills.test.ts
@@ -0,0 +1,136 @@
+import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test";
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
+import {
+ deleteManagedSkill,
+ getManagedSkillsDir,
+ MAX_MANAGED_SKILL_BYTES,
+ sanitizeSkillName,
+ toSkillFrontmatter,
+ writeManagedSkill,
+} from "@oh-my-pi/pi-coding-agent/autolearn/managed-skills";
+import { parseFrontmatter } from "@oh-my-pi/pi-utils";
+
+describe("managed-skills primitives", () => {
+ let tempHome: string;
+
+ beforeEach(async () => {
+ tempHome = await fs.mkdtemp(path.join(os.tmpdir(), "omp-managed-skills-"));
+ spyOn(os, "homedir").mockReturnValue(tempHome);
+ });
+
+ afterEach(async () => {
+ spyOn(os, "homedir").mockRestore();
+ await fs.rm(tempHome, { recursive: true, force: true });
+ });
+
+ const skillFile = (name: string) => path.join(getManagedSkillsDir(), name, "SKILL.md");
+
+ describe("sanitizeSkillName", () => {
+ it("rejects traversal, slashes, and empty names", () => {
+ expect(() => sanitizeSkillName("../escape")).toThrow();
+ expect(() => sanitizeSkillName("a/b")).toThrow();
+ expect(() => sanitizeSkillName("")).toThrow();
+ expect(() => sanitizeSkillName("has space")).toThrow();
+ });
+
+ it("normalizes and accepts a valid kebab name", () => {
+ expect(sanitizeSkillName(" Demo-Skill ")).toBe("demo-skill");
+ });
+ });
+
+ describe("toSkillFrontmatter", () => {
+ it("round-trips name and a description with a quote + newline through parseFrontmatter", () => {
+ const content = `${toSkillFrontmatter("demo", 'has a "quote"\nand newline')}\nbody`;
+ const { frontmatter } = parseFrontmatter(content, { source: "test" });
+ expect(frontmatter.name).toBe("demo");
+ expect(frontmatter.description).toBe('has a "quote" and newline');
+ });
+ });
+
+ describe("writeManagedSkill", () => {
+ it("creates a parseable SKILL.md and rejects a duplicate create", async () => {
+ await writeManagedSkill({ action: "create", name: "foo", description: "When to foo.", body: "# Foo\nbody" });
+ const content = await Bun.file(skillFile("foo")).text();
+ const { frontmatter, body } = parseFrontmatter(content, { source: "test" });
+ expect(frontmatter.name).toBe("foo");
+ expect(frontmatter.description).toBe("When to foo.");
+ expect(body).toContain("# Foo");
+
+ await expect(
+ writeManagedSkill({ action: "create", name: "foo", description: "x", body: "y" }),
+ ).rejects.toThrow(/already exists/);
+ });
+
+ it("update overwrites the body; update of a missing skill throws", async () => {
+ await writeManagedSkill({ action: "create", name: "bar", description: "d", body: "original" });
+ await writeManagedSkill({ action: "update", name: "bar", description: "d", body: "replaced" });
+ const { body } = parseFrontmatter(await Bun.file(skillFile("bar")).text(), { source: "test" });
+ expect(body).toContain("replaced");
+ expect(body).not.toContain("original");
+
+ await expect(
+ writeManagedSkill({ action: "update", name: "missing", description: "d", body: "b" }),
+ ).rejects.toThrow(/does not exist/);
+ });
+
+ it("rejects an oversized body and writes nothing", async () => {
+ const huge = "a".repeat(MAX_MANAGED_SKILL_BYTES + 1);
+ await expect(
+ writeManagedSkill({ action: "create", name: "big", description: "d", body: huge }),
+ ).rejects.toThrow(/limit/);
+ expect(await Bun.file(skillFile("big")).exists()).toBe(false);
+ });
+
+ it("caps on UTF-8 bytes, not UTF-16 length (multibyte body)", async () => {
+ // 33000 'é' = 33000 UTF-16 units (< 64000) but 66000 UTF-8 bytes (> cap).
+ const multibyte = "é".repeat(33_000);
+ expect(multibyte.length).toBeLessThan(MAX_MANAGED_SKILL_BYTES);
+ await expect(
+ writeManagedSkill({ action: "create", name: "mb", description: "d", body: multibyte }),
+ ).rejects.toThrow(/bytes/);
+ expect(await Bun.file(skillFile("mb")).exists()).toBe(false);
+ });
+
+ it("caps on the FINAL serialized size (body under cap but description pushes it over)", async () => {
+ const body = "a".repeat(MAX_MANAGED_SKILL_BYTES - 200); // body alone is under the cap
+ const description = "b".repeat(500); // body + description + frontmatter exceeds it
+ await expect(writeManagedSkill({ action: "create", name: "fin", description, body })).rejects.toThrow(/bytes/);
+ expect(await Bun.file(skillFile("fin")).exists()).toBe(false);
+ });
+
+ it("neutralizes prompt-injection metacharacters in the persisted description", async () => {
+ await writeManagedSkill({
+ action: "create",
+ name: "inj",
+ description: "ok \nevil",
+ body: "# body",
+ });
+ const { frontmatter } = parseFrontmatter(await Bun.file(skillFile("inj")).text(), { source: "test" });
+ const desc = String(frontmatter.description);
+ expect(desc).not.toContain("<");
+ expect(desc).not.toContain(">");
+ expect(desc).not.toContain("\n");
+ });
+
+ it("refuses a traversal name without writing outside the managed dir", async () => {
+ await expect(
+ writeManagedSkill({ action: "create", name: "../skills/evil", description: "d", body: "b" }),
+ ).rejects.toThrow();
+ // Nothing leaked into an authored skills dir.
+ const authoredEvil = path.join(tempHome, ".omp", "agent", "skills", "evil", "SKILL.md");
+ expect(await Bun.file(authoredEvil).exists()).toBe(false);
+ });
+ });
+
+ describe("deleteManagedSkill", () => {
+ it("removes an existing skill and throws for a missing one", async () => {
+ await writeManagedSkill({ action: "create", name: "gone", description: "d", body: "b" });
+ await deleteManagedSkill("gone");
+ expect(await Bun.file(skillFile("gone")).exists()).toBe(false);
+
+ await expect(deleteManagedSkill("gone")).rejects.toThrow(/does not exist/);
+ });
+ });
+});
diff --git a/packages/coding-agent/test/autolearn-tools-gating.test.ts b/packages/coding-agent/test/autolearn-tools-gating.test.ts
new file mode 100644
index 000000000..8f2034b23
--- /dev/null
+++ b/packages/coding-agent/test/autolearn-tools-gating.test.ts
@@ -0,0 +1,164 @@
+import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test";
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
+import { getManagedSkillsDir } from "@oh-my-pi/pi-coding-agent/autolearn/managed-skills";
+import { type SettingPath, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
+import type { MnemopiSessionState } from "@oh-my-pi/pi-coding-agent/mnemopi/state";
+import { createTools, type ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
+import { LearnTool } from "@oh-my-pi/pi-coding-agent/tools/learn";
+import { ManageSkillTool } from "@oh-my-pi/pi-coding-agent/tools/manage-skill";
+
+Bun.env.PI_PYTHON_SKIP_CHECK = "1";
+
+function makeSession(
+ settingsOverrides: Partial> = {},
+ extra: Partial = {},
+): ToolSession {
+ return {
+ cwd: "/tmp/test",
+ hasUI: false,
+ skipPythonPreflight: true,
+ getSessionFile: () => null,
+ getSessionSpawns: () => "*",
+ settings: Settings.isolated(settingsOverrides),
+ ...extra,
+ };
+}
+
+describe("autolearn tool gating", () => {
+ afterEach(() => {
+ spyOn(os, "homedir").mockRestore();
+ });
+
+ it("offers neither tool by default (autolearn disabled)", async () => {
+ const names = (await createTools(makeSession())).map(t => t.name);
+ expect(names).not.toContain("learn");
+ expect(names).not.toContain("manage_skill");
+ });
+
+ it("offers manage_skill but not learn when enabled with no memory backend", async () => {
+ const names = (await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "off" }))).map(
+ t => t.name,
+ );
+ expect(names).toContain("manage_skill");
+ expect(names).not.toContain("learn");
+ });
+
+ it("offers both tools, marked essential, when enabled with a live backend", async () => {
+ const tools = await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "mnemopi" }));
+ const learn = tools.find(t => t.name === "learn");
+ const manage = tools.find(t => t.name === "manage_skill");
+ expect(learn).toBeDefined();
+ expect(manage).toBeDefined();
+ // loadMode "essential" is what keeps them active under tools.discoveryMode "all".
+ expect(learn?.loadMode).toBe("essential");
+ expect(manage?.loadMode).toBe("essential");
+ });
+
+ it("force-includes the tools into an explicit restricted toolNames list", async () => {
+ // A session created with autolearn on but a narrow tool list still gets the
+ // controller/guidance, so the tools the nudge points at must be present.
+ const withBackend = (
+ await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "mnemopi" }), ["read"])
+ ).map(t => t.name);
+ expect(withBackend).toContain("manage_skill");
+ expect(withBackend).toContain("learn");
+
+ const noBackend = (
+ await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "off" }), ["read"])
+ ).map(t => t.name);
+ expect(noBackend).toContain("manage_skill");
+ expect(noBackend).not.toContain("learn");
+ });
+});
+
+describe("manage_skill execute", () => {
+ let tempHome: string;
+
+ beforeEach(async () => {
+ tempHome = await fs.mkdtemp(path.join(os.tmpdir(), "omp-manage-skill-"));
+ spyOn(os, "homedir").mockReturnValue(tempHome);
+ });
+
+ afterEach(async () => {
+ spyOn(os, "homedir").mockRestore();
+ await fs.rm(tempHome, { recursive: true, force: true });
+ });
+
+ const tool = () => ManageSkillTool.createIf(makeSession({ "autolearn.enabled": true }))!;
+
+ it("create writes the managed SKILL.md; delete removes it", async () => {
+ const file = path.join(getManagedSkillsDir(), "demo", "SKILL.md");
+ await tool().execute("1", { action: "create", name: "demo", description: "When to demo.", body: "# Demo" });
+ expect(await Bun.file(file).exists()).toBe(true);
+
+ await tool().execute("2", { action: "delete", name: "demo" });
+ expect(await Bun.file(file).exists()).toBe(false);
+ });
+
+ it("rejects create without a body and delete of a missing skill", async () => {
+ await expect(tool().execute("3", { action: "create", name: "nobody", description: "d" })).rejects.toThrow(
+ /requires/,
+ );
+ await expect(tool().execute("4", { action: "delete", name: "absent" })).rejects.toThrow(/does not exist/);
+ });
+});
+
+describe("learn execute", () => {
+ let tempHome: string;
+ let remembered: string[];
+
+ function learnSession(): ToolSession {
+ const fakeState = {
+ sessionId: "sess-1",
+ session: { sessionManager: { getCwd: () => "/tmp/work" } },
+ rememberScoped: (memory: string) => {
+ remembered.push(memory);
+ return "mem-id";
+ },
+ };
+ return makeSession(
+ { "autolearn.enabled": true, "memory.backend": "mnemopi" },
+ { getMnemopiSessionState: () => fakeState as unknown as MnemopiSessionState },
+ );
+ }
+
+ beforeEach(async () => {
+ tempHome = await fs.mkdtemp(path.join(os.tmpdir(), "omp-learn-"));
+ spyOn(os, "homedir").mockReturnValue(tempHome);
+ remembered = [];
+ });
+
+ afterEach(async () => {
+ spyOn(os, "homedir").mockRestore();
+ await fs.rm(tempHome, { recursive: true, force: true });
+ });
+
+ it("stores a lesson to memory without writing a skill when no skill payload", async () => {
+ await new LearnTool(learnSession()).execute("1", { memory: "Prefer Bun.file over readFileSync." });
+ expect(remembered).toEqual(["Prefer Bun.file over readFileSync."]);
+ // No managed skills written.
+ expect(await fs.readdir(getManagedSkillsDir()).catch(() => [])).toHaveLength(0);
+ });
+
+ it("stores a lesson AND mints a managed skill when a skill payload is given", async () => {
+ await new LearnTool(learnSession()).execute("2", {
+ memory: "Use the worker host entry pattern.",
+ skill: { action: "create", name: "worker-host", description: "Spawn workers.", body: "# Worker host" },
+ });
+ expect(remembered).toHaveLength(1);
+ expect(await Bun.file(path.join(getManagedSkillsDir(), "worker-host", "SKILL.md")).exists()).toBe(true);
+ });
+
+ it("surfaces a partial-outcome error when the skill name is invalid", async () => {
+ await expect(
+ new LearnTool(learnSession()).execute("3", {
+ memory: "lesson",
+ skill: { action: "create", name: "../evil", description: "d", body: "b" },
+ }),
+ ).rejects.toThrow(/Lesson stored, but the managed skill could not be written/);
+ // The memory half still ran.
+ expect(remembered).toHaveLength(1);
+ });
+});
diff --git a/packages/coding-agent/test/tool-discovery/initial-tools.test.ts b/packages/coding-agent/test/tool-discovery/initial-tools.test.ts
index 635dcec20..d174338d1 100644
--- a/packages/coding-agent/test/tool-discovery/initial-tools.test.ts
+++ b/packages/coding-agent/test/tool-discovery/initial-tools.test.ts
@@ -28,6 +28,7 @@ const allToolsSettings = Settings.isolated({
"checkpoint.enabled": true,
"todo.enabled": true,
"memory.backend": "mnemopi",
+ "autolearn.enabled": true,
"tools.discoveryMode": "all",
});
From 9662844853da3fe7e536fe590c3da62c22e712e9 Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 12:03:13 +0900
Subject: [PATCH 02/14] feat(coding-agent): support the local memory backend in
the learn tool
The `learn` tool previously required a `hindsight`/`mnemopi` backend. It now
also works when `memory.backend` is `local` (the file-based rollout backend):
lessons append to a `learned.md` under the project's memory root, kept separate
from the consolidation artifacts so a consolidation pass never clobbers them,
and are injected into future sessions alongside the memory summary.
- memories: `saveLearnedLesson` (newest-first, deduped, count- and per-field
size-capped, secret-redacted, injection-neutralized) with per-path write
serialization; `buildMemoryToolDeveloperInstructions` reads `learned.md` and
shares one injection budget with the summary; `redactSecrets` extended with
GitHub/npm/Slack/Google token prefixes.
- local backend: implements `save()`; status reports `writable: true`.
- learn tool: `local` execute branch; `createIf`/`isToolAllowed`/auto-include
and the standing guidance extended to `local`; local saves tier as a `write`
approval.
- read-path prompt: renders the learned-lessons block when present.
- Lessons are injection-neutralized and secret-redacted on BOTH write and read
(they render unescaped into the system prompt).
Also moves the auto-learn CHANGELOG entry out of the released [15.12.6] section
(a cherry-pick artifact) back under [Unreleased] and notes the local backend.
Tests: local storage (format, dedup, cap, redaction incl. provider/delimiter-
split tokens, concurrency), read-back (with/without summary, off-gating, raw
hand-edited file), tool gating + write-approval tiering.
---
packages/coding-agent/CHANGELOG.md | 2 +-
.../coding-agent/src/autolearn/controller.ts | 5 +-
packages/coding-agent/src/memories/index.ts | 152 ++++++++++-
.../src/memory-backend/local-backend.ts | 16 +-
.../src/prompts/memories/read-path.md | 6 +
packages/coding-agent/src/tools/index.ts | 4 +-
packages/coding-agent/src/tools/learn.ts | 19 +-
.../test/autolearn-controller.test.ts | 8 +
.../test/autolearn-learn-local.test.ts | 249 ++++++++++++++++++
.../test/autolearn-tools-gating.test.ts | 14 +
.../test/memory-backend-resolve.test.ts | 4 +-
11 files changed, 454 insertions(+), 25 deletions(-)
create mode 100644 packages/coding-agent/test/autolearn-learn-local.test.ts
diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md
index f207171e2..5a1b80724 100644
--- a/packages/coding-agent/CHANGELOG.md
+++ b/packages/coding-agent/CHANGELOG.md
@@ -9,6 +9,7 @@
- Added a `fastModeScope` setting (`both` | `openai` | `claude`, default `both`) controlling which providers `/fast on` (and the fast-mode toggle) target. `both` keeps the prior unscoped priority behavior; `openai`/`claude` scope fast mode to one family. `/fast status` now reports the active scope.
- Added the `mnemopi.embeddingVariant` setting (`en` | `multilingual`) selecting a stronger SOTA local embedding model — `en` → `BAAI/bge-base-en-v1.5` (768d), `multilingual` → `intfloat/multilingual-e5-large` (1024d). Resolution precedence is `mnemopi.embeddingModel` setting > `MNEMOPI_EMBEDDING_MODEL` env > variant default, so the documented env override is still honored. Changing the active model wipes and rebuilds stored embeddings on the next writable start ([#2476](https://github.com/can1357/oh-my-pi/issues/2476))
- Added a `/guided-goal` slash command that interviews you to refine an objective before enabling goal mode, then seeds goal mode with the agreed objective. The bounded interview (up to six turns) runs on the plan or slow model and falls back with a hint when the goal is still too vague ([#2502](https://github.com/can1357/oh-my-pi/issues/2502)).
+- Added an experimental, opt-in **auto-learn** loop (default off, `autolearn.enabled`). When enabled, after the agent stops it is nudged to capture reusable lessons: durable facts go to long-term memory and repeatable procedures become **managed skills** — `SKILL.md` files written to an isolated `~/.omp/agent/managed-skills` directory that is discovered and surfaced like authored skills but never overwrites user-authored skills (authored names always win). Two tools back this: `manage_skill` (create/update/delete managed skills) and `learn` (record a lesson, optionally minting a managed skill in the same call). `learn` works with the `hindsight`, `mnemopi`, or file-based `local` memory backend; under `local`, lessons append to a `learned.md` in the project's memory root (kept separate from the consolidation artifacts so they survive a consolidation pass) and are injected into future sessions. The nudge is passive by default (a hidden reminder rides the next turn); `autolearn.autoContinue` instead auto-runs one capture turn at stop, and `autolearn.minToolCalls` (default 5) gates trivial turns. Plan/goal-mode turns and subagents are never nudged.
### Fixed
@@ -40,7 +41,6 @@
- Changed session persistence internals to expose `writeTextAtomic(...)` on session storage writers for atomic whole-file replacements
- Changed online session-title generation to support tool-choice-less title models. Providers/models that cannot be forced to call a tool (chat-completions hosts without `tool_choice` support such as DeepSeek V4, and Claude Fable/Mythos) are now prompted to wrap the title in `...` markers instead of the `set_title` tool call; extraction is lenient, accepting a plain sentence or a truncated/unclosed tag. A `TITLE_SYSTEM.md` override is reused in this mode with the marker instruction appended.
-- Added an experimental, opt-in **auto-learn** loop (default off, `autolearn.enabled`). When enabled, after the agent stops it is nudged to capture reusable lessons: durable facts go to long-term memory and repeatable procedures become **managed skills** — `SKILL.md` files written to an isolated `~/.omp/agent/managed-skills` directory that is discovered and surfaced like authored skills but never overwrites user-authored skills (authored names always win). Two tools back this: `manage_skill` (create/update/delete managed skills) and `learn` (record a lesson, optionally minting a managed skill in the same call; requires a `hindsight`/`mnemopi` memory backend). The nudge is passive by default (a hidden reminder rides the next turn); `autolearn.autoContinue` instead auto-runs one capture turn at stop, and `autolearn.minToolCalls` (default 5) gates trivial turns. Plan/goal-mode turns and subagents are never nudged.
### Fixed
diff --git a/packages/coding-agent/src/autolearn/controller.ts b/packages/coding-agent/src/autolearn/controller.ts
index 2bdf2dfe8..07c3d6ba2 100644
--- a/packages/coding-agent/src/autolearn/controller.ts
+++ b/packages/coding-agent/src/autolearn/controller.ts
@@ -23,11 +23,12 @@ const DEFAULT_MIN_TOOL_CALLS = 5;
/**
* Build the standing auto-learn guidance for the system prompt, or null when
* the feature is disabled. The `learn` addendum is included only when a memory
- * backend is live (the `learn` tool requires one).
+ * backend is live (the `learn` tool requires one — `hindsight`, `mnemopi`, or
+ * the file-based `local` backend).
*/
export function buildAutoLearnInstructions(settings: Settings): string | null {
if (!settings.get("autolearn.enabled")) return null;
- const learnEnabled = ["hindsight", "mnemopi"].includes(settings.get("memory.backend") ?? "");
+ const learnEnabled = ["hindsight", "mnemopi", "local"].includes(settings.get("memory.backend") ?? "");
const parts = [autolearnGuidance.trim()];
if (learnEnabled) parts.push(autolearnGuidanceLearn.trim());
return parts.join("\n\n");
diff --git a/packages/coding-agent/src/memories/index.ts b/packages/coding-agent/src/memories/index.ts
index a5fb2dff3..eabcc28cb 100644
--- a/packages/coding-agent/src/memories/index.ts
+++ b/packages/coding-agent/src/memories/index.ts
@@ -5,11 +5,12 @@ import * as path from "node:path";
import type { AgentMessage } from "@oh-my-pi/pi-agent-core";
import { type ApiKey, completeSimple, Effort, type Model } from "@oh-my-pi/pi-ai";
import { clampThinkingLevelForModel } from "@oh-my-pi/pi-catalog/model-thinking";
-import { getAgentDbPath, getMemoriesDir, logger, parseJsonlLenient, prompt } from "@oh-my-pi/pi-utils";
+import { getAgentDbPath, getMemoriesDir, isEnoent, logger, parseJsonlLenient, prompt } from "@oh-my-pi/pi-utils";
import type { ModelRegistry } from "../config/model-registry";
import { getModelMatchPreferences, resolveModelRoleValue } from "../config/model-resolver";
import type { Settings } from "../config/settings";
+import type { MemoryBackendSaveInput, MemoryBackendSaveResult } from "../memory-backend/types";
import consolidationTemplate from "../prompts/memories/consolidation.md" with { type: "text" };
import consolidationSystemTemplate from "../prompts/memories/consolidation_system.md" with { type: "text" };
import readPathTemplate from "../prompts/memories/read-path.md" with { type: "text" };
@@ -156,22 +157,28 @@ export async function buildMemoryToolDeveloperInstructions(
const cfg = loadMemoryConfig(settings);
if (!cfg.enabled) return undefined;
const memoryRoot = getMemoryRoot(agentDir, settings.getCwd());
- const summaryPath = path.join(memoryRoot, "memory_summary.md");
- let text: string;
+ let summary = "";
try {
- text = await Bun.file(summaryPath).text();
+ summary = (await Bun.file(path.join(memoryRoot, "memory_summary.md")).text()).trim();
} catch {
- return undefined;
+ // Missing or unreadable summary — injection is best-effort; fall through
+ // so any captured lessons still surface on their own.
}
+ const learned = await readLearnedLessons(memoryRoot);
+ if (!summary && !learned) return undefined;
- const summary = text.trim();
- if (!summary) return undefined;
- const truncated = truncateByApproxTokens(summary, cfg.summaryInjectionTokenLimit);
- if (!truncated.trim()) return undefined;
+ const summaryOut = summary ? truncateByApproxTokens(summary, cfg.summaryInjectionTokenLimit).trim() : "";
+ // Lessons share ONE injection budget with the summary so the combined block
+ // stays within `summaryInjectionTokenLimit` (~4 chars/token, matching
+ // truncateByApproxTokens). With no summary, lessons get the whole budget.
+ const learnedBudget = cfg.summaryInjectionTokenLimit - Math.ceil(summaryOut.length / 4);
+ const learnedOut = learned ? truncateByApproxTokens(learned, learnedBudget).trim() : "";
+ if (!summaryOut && !learnedOut) return undefined;
return prompt.render(readPathTemplate, {
- memory_summary: truncated,
+ memory_summary: summaryOut,
+ learned: learnedOut,
});
}
@@ -982,6 +989,12 @@ function redactSecrets(input: string): string {
/(?:sk|pk|rk|tok|key|secret|token|password)[-_A-Za-z0-9]{12,}/g,
/[A-Za-z0-9_-]{16,}\.[A-Za-z0-9_-]{16,}\.[A-Za-z0-9_-]{16,}/g,
/(?:AKIA|ASIA)[A-Z0-9]{16}/g,
+ // Common provider token prefixes (GitHub, npm, Slack, Google).
+ /(?:ghp|gho|ghu|ghs|ghr)_[A-Za-z0-9]{20,}/g,
+ /github_pat_[A-Za-z0-9_]{20,}/g,
+ /npm_[A-Za-z0-9]{30,}/g,
+ /xox[baprs]-[A-Za-z0-9-]{10,}/g,
+ /AIza[A-Za-z0-9_-]{30,}/g,
];
for (const pattern of patterns) {
out = out.replace(pattern, "[REDACTED]");
@@ -1121,6 +1134,125 @@ export function getMemoryRoot(agentDir: string, cwd: string): string {
return path.join(getMemoriesDir(agentDir), encodeProjectPath(cwd));
}
+/**
+ * Filename of the captured-lessons file under a project's memory root.
+ *
+ * Written by the `learn` tool via {@link saveLearnedLesson} and read back by
+ * {@link buildMemoryToolDeveloperInstructions}. Deliberately distinct from the
+ * consolidation artifacts (`MEMORY.md`, `memory_summary.md`, `skills/`) so a
+ * consolidation pass never clobbers manually captured lessons.
+ */
+const LEARNED_LESSONS_FILE = "learned.md";
+/** Newest-first cap on retained lessons, bounding file growth by entry count. */
+const MAX_LEARNED_LESSONS = 100;
+/** Per-field char caps so a single huge capture can't bloat learned.md. */
+const MAX_LEARNED_CONTENT_CHARS = 2000;
+const MAX_LEARNED_CONTEXT_CHARS = 400;
+
+/**
+ * Strip prompt-injection vectors from a single line of lesson text: control/
+ * format chars, angle brackets (``), backticks, and `~~~` fences, then
+ * collapse whitespace. Applied on BOTH write and read (the block renders
+ * unescaped into the system prompt), mirroring managed-skill descriptions.
+ */
+function neutralizeInjection(text: string): string {
+ return text
+ .replace(/[\p{Cc}\p{Cf}]/gu, " ")
+ .replace(/[<>`]/g, "")
+ .replace(/~{2,}/g, "~")
+ .replace(/\s+/g, " ")
+ .trim();
+}
+
+/** Slice to `maxChars`, dropping a trailing unpaired high surrogate. */
+function boundChars(text: string, maxChars: number): string {
+ if (text.length <= maxChars) return text;
+ const sliced = text.slice(0, maxChars);
+ return /[\uD800-\uDBFF]$/.test(sliced) ? sliced.slice(0, -1) : sliced;
+}
+
+/**
+ * Normalize one lesson field for storage: neutralize injection delimiters
+ * FIRST, then redact secrets (so delimiter stripping can't reassemble a token
+ * the redactor would have caught), then bound the length.
+ */
+function normalizeLearnedText(text: string, maxChars: number): string {
+ return boundChars(redactSecrets(neutralizeInjection(text)).trim(), maxChars);
+}
+
+/** Per-path write chains serializing `learned.md` read-modify-write. */
+const learnedWriteChains = new Map>();
+
+/**
+ * Append one lesson to the project's `learned.md` (newest-first, deduped,
+ * capped, secret-redacted, injection-neutralized). The file backs the `learn`
+ * tool when `memory.backend` is `local`.
+ */
+export async function saveLearnedLesson(
+ agentDir: string,
+ cwd: string,
+ input: MemoryBackendSaveInput,
+): Promise {
+ const content = normalizeLearnedText(input.content, MAX_LEARNED_CONTENT_CHARS);
+ if (!content) {
+ return { backend: "local", stored: 0, message: "Empty lesson; nothing stored." };
+ }
+ const context = input.context ? normalizeLearnedText(input.context, MAX_LEARNED_CONTEXT_CHARS) : "";
+ const line = context ? `- ${content} _(context: ${context})_` : `- ${content}`;
+ const filePath = path.join(getMemoryRoot(agentDir, cwd), LEARNED_LESSONS_FILE);
+
+ // Serialize the read-modify-write per file: parallel `learn` calls (sibling
+ // subagents, or two shared tool calls in one turn) share the project memory
+ // root, so an unguarded RMW would let the last writer drop the other's lesson.
+ const run = (learnedWriteChains.get(filePath) ?? Promise.resolve()).then(() => appendLearnedLine(filePath, line));
+ const guarded = run.catch(() => {});
+ learnedWriteChains.set(filePath, guarded);
+ try {
+ await run;
+ } finally {
+ // Drop the entry once this write is the chain tail, so the map does not
+ // retain one promise per distinct memory root for the process lifetime.
+ if (learnedWriteChains.get(filePath) === guarded) learnedWriteChains.delete(filePath);
+ }
+ return { backend: "local", stored: 1, message: `Lesson saved to ${LEARNED_LESSONS_FILE}.` };
+}
+
+async function appendLearnedLine(filePath: string, line: string): Promise {
+ let existing = "";
+ try {
+ existing = await Bun.file(filePath).text();
+ } catch (err) {
+ if (!isEnoent(err)) throw err;
+ }
+ const prior = existing
+ .split("\n")
+ .map(l => l.trim())
+ .filter(l => l.startsWith("- ") && l !== line);
+ const lessons = [line, ...prior].slice(0, MAX_LEARNED_LESSONS);
+ await Bun.write(filePath, `${lessons.join("\n")}\n`);
+}
+
+/**
+ * Read `learned.md`, neutralizing each line on read too — a hand-edited or
+ * pre-existing file bypasses write-time normalization and the block renders
+ * unescaped into the system prompt. Returns "" when absent/unreadable.
+ */
+async function readLearnedLessons(memoryRoot: string): Promise {
+ let raw = "";
+ try {
+ raw = (await Bun.file(path.join(memoryRoot, LEARNED_LESSONS_FILE)).text()).trim();
+ } catch {
+ return "";
+ }
+ if (!raw) return "";
+ // Neutralize delimiters THEN redact per line — mirrors the write path so a
+ // hand-edited line cannot reassemble a token after delimiter stripping.
+ return raw
+ .split("\n")
+ .map(line => redactSecrets(neutralizeInjection(line)))
+ .join("\n");
+}
+
function encodeProjectPath(cwd: string): string {
return `--${cwd.replace(/^[/\\]/, "").replace(/[/\\:]/g, "-")}--`;
}
diff --git a/packages/coding-agent/src/memory-backend/local-backend.ts b/packages/coding-agent/src/memory-backend/local-backend.ts
index e36c7145d..0c7726ddb 100644
--- a/packages/coding-agent/src/memory-backend/local-backend.ts
+++ b/packages/coding-agent/src/memory-backend/local-backend.ts
@@ -2,6 +2,7 @@ import {
buildMemoryToolDeveloperInstructions,
clearMemoryData,
enqueueMemoryConsolidation,
+ saveLearnedLesson,
startMemoryStartupTask,
} from "../memories";
import type { MemoryBackend } from "./types";
@@ -9,9 +10,10 @@ import type { MemoryBackend } from "./types";
/**
* Wraps the existing `memories/` module as a `MemoryBackend`.
*
- * No behavioural change — every call delegates to the legacy entry points so
- * the local memory pipeline (rollout summarisation → SQLite → memory_summary.md)
- * keeps working exactly as before.
+ * The rollout-summarisation pipeline (rollouts → SQLite → memory_summary.md) is
+ * delegated unchanged. On top of it, `save()` persists `learn`-tool lessons to
+ * `learned.md` (so `status()` reports `writable: true`); structured search is
+ * still unavailable.
*/
export const localBackend: MemoryBackend = {
id: "local",
@@ -27,13 +29,17 @@ export const localBackend: MemoryBackend = {
async enqueue(agentDir, cwd) {
enqueueMemoryConsolidation(agentDir, cwd);
},
+ async save(context, input) {
+ return saveLearnedLesson(context.agentDir, context.cwd, input);
+ },
async status() {
return {
backend: "local" as const,
active: true,
- writable: false,
+ writable: true,
searchable: false,
- message: "Local rollout-summary memory is active; structured search/save is not available.",
+ message:
+ "Local rollout-summary memory is active; lessons from the `learn` tool are saved to learned.md. Structured search is not available.",
};
},
};
diff --git a/packages/coding-agent/src/prompts/memories/read-path.md b/packages/coding-agent/src/prompts/memories/read-path.md
index fdc85934f..9ef90dcaa 100644
--- a/packages/coding-agent/src/prompts/memories/read-path.md
+++ b/packages/coding-agent/src/prompts/memories/read-path.md
@@ -7,5 +7,11 @@ Operational rules:
4) When memory changes your plan, cite the artifact path (e.g. `memory://root/skills//SKILL.md`) and pair it with current-repo evidence.
5) If memory disagrees with repo state or user instruction, treat memory as stale: proceed with corrected behavior, then update/regenerate memory artifacts.
6) Escalate confidence only after repository verification. Memory alone is NEVER sufficient proof.
+{{#if memory_summary}}
Memory summary:
{{memory_summary}}
+{{/if}}
+{{#if learned}}
+Learned lessons (captured via the `learn` tool; durable but may be stale — verify against the repo before relying on them):
+{{learned}}
+{{/if}}
diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts
index 22fb54586..9ec7fe955 100644
--- a/packages/coding-agent/src/tools/index.ts
+++ b/packages/coding-agent/src/tools/index.ts
@@ -532,7 +532,7 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P
if (session.settings.get("autolearn.enabled")) {
if (!requestedTools.includes("manage_skill")) requestedTools.push("manage_skill");
if (
- ["hindsight", "mnemopi"].includes(session.settings.get("memory.backend") ?? "") &&
+ ["hindsight", "mnemopi", "local"].includes(session.settings.get("memory.backend") ?? "") &&
!requestedTools.includes("learn")
) {
requestedTools.push("learn");
@@ -575,7 +575,7 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P
if (name === "learn") {
return (
session.settings.get("autolearn.enabled") &&
- ["hindsight", "mnemopi"].includes(session.settings.get("memory.backend") ?? "")
+ ["hindsight", "mnemopi", "local"].includes(session.settings.get("memory.backend") ?? "")
);
}
if (name === "task") {
diff --git a/packages/coding-agent/src/tools/learn.ts b/packages/coding-agent/src/tools/learn.ts
index 9dea74df8..c9197660f 100644
--- a/packages/coding-agent/src/tools/learn.ts
+++ b/packages/coding-agent/src/tools/learn.ts
@@ -1,6 +1,7 @@
import type { AgentTool, AgentToolResult } from "@oh-my-pi/pi-agent-core";
import { z } from "zod/v4";
import { writeManagedSkill } from "../autolearn/managed-skills";
+import { localBackend } from "../memory-backend/local-backend";
import learnDescription from "../prompts/tools/learn.md" with { type: "text" };
import type { ToolSession } from ".";
@@ -24,11 +25,15 @@ export type LearnParams = z.infer;
* Orchestrating "learn" tool: persists a lesson to long-term memory and,
* given a `skill` payload, mints/enhances a managed skill via the shared
* `writeManagedSkill` primitive. Gated behind `autolearn.enabled` plus a live
- * memory backend (the memory half needs one).
+ * memory backend — `hindsight`/`mnemopi` (remote/SQLite) or `local` (the
+ * file-based rollout backend, where lessons append to `learned.md`).
*/
export class LearnTool implements AgentTool {
readonly name = "learn";
- readonly approval = (args: unknown) => ((args as Partial).skill ? "write" : "read");
+ readonly approval = (args: unknown) =>
+ (args as Partial).skill || this.session.settings.get("memory.backend") === "local"
+ ? "write"
+ : "read";
readonly label = "Learn";
readonly description = learnDescription;
readonly parameters = learnSchema;
@@ -41,7 +46,7 @@ export class LearnTool implements AgentTool {
static createIf(session: ToolSession): LearnTool | null {
if (!session.settings.get("autolearn.enabled")) return null;
const backend = session.settings.get("memory.backend");
- if (backend !== "hindsight" && backend !== "mnemopi") return null;
+ if (backend !== "hindsight" && backend !== "mnemopi" && backend !== "local") return null;
return new LearnTool(session);
}
@@ -68,6 +73,14 @@ export class LearnTool implements AgentTool {
veracity: "tool",
memoryType: "fact",
});
+ } else if (backend === "local") {
+ const result = await localBackend.save?.(
+ { agentDir: this.session.settings.getAgentDir(), cwd: this.session.settings.getCwd() },
+ { content: params.memory, context: params.context, source: "coding-agent-learn", importance: 0.8 },
+ );
+ if (!result || result.stored === 0) {
+ throw new Error("Lesson was empty after sanitization; nothing stored.");
+ }
} else {
const state = this.session.getHindsightSessionState?.();
if (!state) {
diff --git a/packages/coding-agent/test/autolearn-controller.test.ts b/packages/coding-agent/test/autolearn-controller.test.ts
index 9b3dc2e83..4e3a75c3f 100644
--- a/packages/coding-agent/test/autolearn-controller.test.ts
+++ b/packages/coding-agent/test/autolearn-controller.test.ts
@@ -183,6 +183,14 @@ describe("buildAutoLearnInstructions", () => {
expect(text).toContain("long-term memory");
});
+ it("includes the learn addendum for the file-based local backend", () => {
+ const text = buildAutoLearnInstructions(
+ Settings.isolated({ "autolearn.enabled": true, "memory.backend": "local" }),
+ );
+ expect(text).toContain("manage_skill");
+ expect(text).toContain("long-term memory");
+ });
+
it("omits the learn addendum when no memory backend is configured", () => {
const text = buildAutoLearnInstructions(
Settings.isolated({ "autolearn.enabled": true, "memory.backend": "off" }),
diff --git a/packages/coding-agent/test/autolearn-learn-local.test.ts b/packages/coding-agent/test/autolearn-learn-local.test.ts
new file mode 100644
index 000000000..d917bb696
--- /dev/null
+++ b/packages/coding-agent/test/autolearn-learn-local.test.ts
@@ -0,0 +1,249 @@
+import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test";
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
+import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
+import {
+ buildMemoryToolDeveloperInstructions,
+ getMemoryRoot,
+ saveLearnedLesson,
+} from "@oh-my-pi/pi-coding-agent/memories";
+import { localBackend } from "@oh-my-pi/pi-coding-agent/memory-backend/local-backend";
+import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
+import { LearnTool } from "@oh-my-pi/pi-coding-agent/tools/learn";
+
+Bun.env.PI_PYTHON_SKIP_CHECK = "1";
+
+describe("learned-lesson storage (local backend)", () => {
+ let tmp: string;
+ let agentDir: string;
+ let projCwd: string;
+ let learnedFile: string;
+
+ beforeEach(async () => {
+ tmp = await fs.mkdtemp(path.join(os.tmpdir(), "omp-learned-"));
+ agentDir = path.join(tmp, "agent");
+ projCwd = path.join(tmp, "proj");
+ learnedFile = path.join(getMemoryRoot(agentDir, projCwd), "learned.md");
+ });
+ afterEach(async () => {
+ await fs.rm(tmp, { recursive: true, force: true });
+ });
+
+ it("appends a bullet, normalizes whitespace, and inlines context", async () => {
+ const result = await saveLearnedLesson(agentDir, projCwd, {
+ content: "Prefer Bun.file\nover\n\nreadFileSync.",
+ context: "from the build",
+ });
+ expect(result.stored).toBe(1);
+ expect(await Bun.file(learnedFile).text()).toBe(
+ "- Prefer Bun.file over readFileSync. _(context: from the build)_\n",
+ );
+ });
+
+ it("redacts secrets, including provider token prefixes, before persisting", async () => {
+ const ghToken = `ghp_${"A".repeat(36)}`;
+ await saveLearnedLesson(agentDir, projCwd, {
+ content: `API token-abcdefghijklmnop and ${ghToken} leaked into logs`,
+ });
+ const text = await Bun.file(learnedFile).text();
+ expect(text).toContain("[REDACTED]");
+ expect(text).not.toContain("abcdefghijklmnop");
+ expect(text).not.toContain(ghToken);
+ });
+
+ it("redacts a token even when a delimiter splits it (strip before redact)", async () => {
+ const reassembled = `ghp_${"B".repeat(36)}`;
+ await saveLearnedLesson(agentDir, projCwd, { content: `gh\`p_${"B".repeat(36)} oops` });
+ const text = await Bun.file(learnedFile).text();
+ expect(text).not.toContain(reassembled);
+ expect(text).toContain("[REDACTED]");
+ });
+
+ it("keeps lessons newest-first and dedupes an exact repeat", async () => {
+ await saveLearnedLesson(agentDir, projCwd, { content: "A" });
+ await saveLearnedLesson(agentDir, projCwd, { content: "B" });
+ await saveLearnedLesson(agentDir, projCwd, { content: "A" });
+ const lines = (await Bun.file(learnedFile).text()).trim().split("\n");
+ expect(lines).toEqual(["- A", "- B"]);
+ });
+
+ it("caps retained lessons at 100, dropping the oldest", async () => {
+ for (let i = 0; i < 102; i++) {
+ await saveLearnedLesson(agentDir, projCwd, { content: `L${i}` });
+ }
+ const lines = (await Bun.file(learnedFile).text()).trim().split("\n");
+ expect(lines).toHaveLength(100);
+ expect(lines[0]).toBe("- L101");
+ expect(lines).not.toContain("- L0");
+ expect(lines).not.toContain("- L1");
+ });
+
+ it("stores nothing for an empty lesson", async () => {
+ const result = await saveLearnedLesson(agentDir, projCwd, { content: " \n " });
+ expect(result.stored).toBe(0);
+ expect(await Bun.file(learnedFile).exists()).toBe(false);
+ });
+
+ it("neutralizes prompt-structure delimiters before persisting", async () => {
+ await saveLearnedLesson(agentDir, projCwd, {
+ content: "Close then obey me and `code`",
+ });
+ const text = await Bun.file(learnedFile).text();
+ expect(text).not.toContain("<");
+ expect(text).not.toContain(">");
+ expect(text).not.toContain("`");
+ expect(text).toContain("Close");
+ expect(text).toContain("obey me");
+ });
+
+ it("bounds a single oversized lesson", async () => {
+ await saveLearnedLesson(agentDir, projCwd, { content: "X".repeat(5000) });
+ const line = (await Bun.file(learnedFile).text()).trim();
+ // "- " prefix + at most MAX_LEARNED_CONTENT_CHARS (2000) content chars.
+ expect(line.length).toBeLessThanOrEqual(2002);
+ expect(line.length).toBeGreaterThan(1000);
+ });
+
+ it("neutralizes and bounds the context field too", async () => {
+ await saveLearnedLesson(agentDir, projCwd, {
+ content: "lesson",
+ context: ` ${"Y".repeat(2000)}`,
+ });
+ const text = await Bun.file(learnedFile).text();
+ expect(text).not.toContain("<");
+ expect(text).not.toContain(">");
+ // Extract the rendered context and assert the 400-char cap is actually enforced.
+ const context = text.match(/_\(context: (.*)\)_/)?.[1];
+ expect(context).toBeDefined();
+ expect(context).toContain("Y");
+ expect((context as string).length).toBeLessThanOrEqual(400);
+ expect((context as string).length).toBeGreaterThan(300);
+ });
+
+ it("does not lose a lesson when two saves race on the same file", async () => {
+ await Promise.all([
+ saveLearnedLesson(agentDir, projCwd, { content: "Racer one" }),
+ saveLearnedLesson(agentDir, projCwd, { content: "Racer two" }),
+ ]);
+ const text = await Bun.file(learnedFile).text();
+ expect(text).toContain("- Racer one");
+ expect(text).toContain("- Racer two");
+ });
+
+ it("the local backend's save() delegates to the same file", async () => {
+ const result = await localBackend.save?.({ agentDir, cwd: projCwd }, { content: "Via the backend" });
+ expect(result?.stored).toBe(1);
+ expect(await Bun.file(learnedFile).text()).toContain("- Via the backend");
+ });
+
+ it("local backend status reports writable", async () => {
+ const status = await localBackend.status?.({ agentDir, cwd: projCwd });
+ expect(status?.writable).toBe(true);
+ expect(status?.backend).toBe("local");
+ });
+});
+
+describe("learned-lesson read-back", () => {
+ let tmp: string;
+ let agentDir: string;
+
+ beforeEach(async () => {
+ tmp = await fs.mkdtemp(path.join(os.tmpdir(), "omp-learned-read-"));
+ agentDir = path.join(tmp, "agent");
+ });
+ afterEach(async () => {
+ await fs.rm(tmp, { recursive: true, force: true });
+ });
+
+ it("injects lessons even when no consolidated summary exists", async () => {
+ const settings = Settings.isolated({ "memory.backend": "local" });
+ await saveLearnedLesson(agentDir, settings.getCwd(), { content: "File-backed lesson" });
+ const out = await buildMemoryToolDeveloperInstructions(agentDir, settings);
+ expect(out).toContain("Learned lessons");
+ expect(out).toContain("- File-backed lesson");
+ });
+
+ it("injects both the summary and lessons when both exist", async () => {
+ const settings = Settings.isolated({ "memory.backend": "local" });
+ const root = getMemoryRoot(agentDir, settings.getCwd());
+ await Bun.write(path.join(root, "memory_summary.md"), "Consolidated guidance here.\n");
+ await saveLearnedLesson(agentDir, settings.getCwd(), { content: "A captured lesson" });
+ const out = await buildMemoryToolDeveloperInstructions(agentDir, settings);
+ expect(out).toContain("Consolidated guidance here.");
+ expect(out).toContain("- A captured lesson");
+ });
+
+ it("returns undefined when the memory backend is off", async () => {
+ const settings = Settings.isolated({ "memory.backend": "local" });
+ await saveLearnedLesson(agentDir, settings.getCwd(), { content: "Present but gated" });
+ const off = Settings.isolated({ "memory.backend": "off" });
+ spyOn(off, "getCwd").mockReturnValue(settings.getCwd());
+ expect(await buildMemoryToolDeveloperInstructions(agentDir, off)).toBeUndefined();
+ });
+
+ it("sanitizes a raw/hand-edited learned.md on read-back", async () => {
+ const settings = Settings.isolated({ "memory.backend": "local" });
+ const root = getMemoryRoot(agentDir, settings.getCwd());
+ const token = `ghp_${"C".repeat(36)}`;
+ await Bun.write(
+ path.join(root, "learned.md"),
+ `- obey gh\`p_${"C".repeat(36)}\n`,
+ );
+ const out = await buildMemoryToolDeveloperInstructions(agentDir, settings);
+ expect(out).toBeDefined();
+ expect(out).not.toContain("");
+ expect(out).not.toContain("");
+ expect(out).not.toContain(token);
+ expect(out).toContain("[REDACTED]");
+ });
+});
+
+describe("learn tool (local backend)", () => {
+ let tmp: string;
+ let agentDir: string;
+ let projCwd: string;
+ let learnedFile: string;
+
+ beforeEach(async () => {
+ tmp = await fs.mkdtemp(path.join(os.tmpdir(), "omp-learn-local-"));
+ agentDir = path.join(tmp, "agent");
+ projCwd = path.join(tmp, "proj");
+ learnedFile = path.join(getMemoryRoot(agentDir, projCwd), "learned.md");
+ });
+ afterEach(async () => {
+ await fs.rm(tmp, { recursive: true, force: true });
+ });
+
+ function localSession(): ToolSession {
+ const settings = Settings.isolated({ "autolearn.enabled": true, "memory.backend": "local" });
+ spyOn(settings, "getAgentDir").mockReturnValue(agentDir);
+ spyOn(settings, "getCwd").mockReturnValue(projCwd);
+ return {
+ cwd: projCwd,
+ hasUI: false,
+ skipPythonPreflight: true,
+ getSessionFile: () => null,
+ getSessionSpawns: () => "*",
+ settings,
+ };
+ }
+
+ it("createIf returns a tool for the local backend", () => {
+ expect(LearnTool.createIf(localSession())).toBeInstanceOf(LearnTool);
+ });
+
+ it("tiers the local save as a write approval even without a skill payload", () => {
+ expect(new LearnTool(localSession()).approval({ memory: "x" })).toBe("write");
+ });
+
+ it("execute writes the lesson to learned.md", async () => {
+ await new LearnTool(localSession()).execute("1", { memory: "A local tool lesson" });
+ expect(await Bun.file(learnedFile).text()).toContain("- A local tool lesson");
+ });
+
+ it("execute throws when the lesson is empty after sanitization", async () => {
+ await expect(new LearnTool(localSession()).execute("2", { memory: " " })).rejects.toThrow(/empty/i);
+ expect(await Bun.file(learnedFile).exists()).toBe(false);
+ });
+});
diff --git a/packages/coding-agent/test/autolearn-tools-gating.test.ts b/packages/coding-agent/test/autolearn-tools-gating.test.ts
index 8f2034b23..bb59f968c 100644
--- a/packages/coding-agent/test/autolearn-tools-gating.test.ts
+++ b/packages/coding-agent/test/autolearn-tools-gating.test.ts
@@ -71,6 +71,20 @@ describe("autolearn tool gating", () => {
expect(noBackend).toContain("manage_skill");
expect(noBackend).not.toContain("learn");
});
+
+ it("offers learn with the file-based local backend", async () => {
+ const names = (await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "local" }))).map(
+ t => t.name,
+ );
+ expect(names).toContain("learn");
+ expect(names).toContain("manage_skill");
+
+ // Force-included into an explicit restricted toolNames list too.
+ const restricted = (
+ await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "local" }), ["read"])
+ ).map(t => t.name);
+ expect(restricted).toContain("learn");
+ });
});
describe("manage_skill execute", () => {
diff --git a/packages/coding-agent/test/memory-backend-resolve.test.ts b/packages/coding-agent/test/memory-backend-resolve.test.ts
index a46fe005c..576fd288d 100644
--- a/packages/coding-agent/test/memory-backend-resolve.test.ts
+++ b/packages/coding-agent/test/memory-backend-resolve.test.ts
@@ -29,7 +29,7 @@ describe("resolveMemoryBackend", () => {
});
});
- it("reports local backend runtime status without structured search/save support", async () => {
+ it("reports local backend runtime status as writable (lessons) without structured search", async () => {
const settings = Settings.isolated({ "memory.backend": "local" });
const memory = createMemoryRuntimeContext({
agentDir: "/tmp/agent",
@@ -40,7 +40,7 @@ describe("resolveMemoryBackend", () => {
await expect(memory.status()).resolves.toMatchObject({
backend: "local",
active: true,
- writable: false,
+ writable: true,
searchable: false,
});
await expect(memory.search("project preference")).resolves.toMatchObject({
From 34c7f106d904fe0edce7fc7f6a22c874cb8ca934 Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 12:36:31 +0900
Subject: [PATCH 03/14] fix(coding-agent): restrict auto-learn tools to
top-level sessions
The `manage_skill`/`learn` force-include and `isToolAllowed` gates only
checked `autolearn.enabled`, not session depth. A subagent created with an
explicit `tools:[...]` whitelist (which runs `approvalMode: "yolo"`) would
silently gain write-capable tools that can mutate `~/.omp/agent/managed-skills`.
The auto-learn controller only runs for top-level sessions, so gate both the
force-include and `isToolAllowed` for `manage_skill`/`learn` to `taskDepth === 0`.
Also tidies the gating test file: drop a module-level `Bun.env` mutation that
was never restored (the test runner skips the Python preflight already) and a
no-op `afterEach` homedir spy in a block that never mocks homedir. Adds a
subagent-exclusion test.
Addresses review threads on PR #2542 (threads 2, 3, 8, 16).
---
packages/coding-agent/src/tools/index.ts | 11 +++++---
.../test/autolearn-tools-gating.test.ts | 25 ++++++++++++++-----
2 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts
index 9ec7fe955..6b5256433 100644
--- a/packages/coding-agent/src/tools/index.ts
+++ b/packages/coding-agent/src/tools/index.ts
@@ -527,9 +527,11 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P
}
// Auto-learn tools are gated by `autolearn.enabled` but, like the memory
// tools above, must also be force-included into an explicit requestedTools
- // list so a restricted session whose controller/guidance is active still
- // exposes the tools the nudge points at.
- if (session.settings.get("autolearn.enabled")) {
+ // list so a restricted top-level session whose controller/guidance is
+ // active still exposes the tools the nudge points at. Gated to top-level
+ // (taskDepth 0): the controller only runs there, so a subagent's explicit
+ // tool whitelist must never be silently widened with write-capable tools.
+ if (session.settings.get("autolearn.enabled") && (session.taskDepth ?? 0) === 0) {
if (!requestedTools.includes("manage_skill")) requestedTools.push("manage_skill");
if (
["hindsight", "mnemopi", "local"].includes(session.settings.get("memory.backend") ?? "") &&
@@ -571,10 +573,11 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P
if (name === "retain" || name === "recall" || name === "reflect") {
return ["hindsight", "mnemopi"].includes(session.settings.get("memory.backend") ?? "");
}
- if (name === "manage_skill") return session.settings.get("autolearn.enabled");
+ if (name === "manage_skill") return session.settings.get("autolearn.enabled") && (session.taskDepth ?? 0) === 0;
if (name === "learn") {
return (
session.settings.get("autolearn.enabled") &&
+ (session.taskDepth ?? 0) === 0 &&
["hindsight", "mnemopi", "local"].includes(session.settings.get("memory.backend") ?? "")
);
}
diff --git a/packages/coding-agent/test/autolearn-tools-gating.test.ts b/packages/coding-agent/test/autolearn-tools-gating.test.ts
index bb59f968c..ab98f7288 100644
--- a/packages/coding-agent/test/autolearn-tools-gating.test.ts
+++ b/packages/coding-agent/test/autolearn-tools-gating.test.ts
@@ -9,8 +9,6 @@ import { createTools, type ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
import { LearnTool } from "@oh-my-pi/pi-coding-agent/tools/learn";
import { ManageSkillTool } from "@oh-my-pi/pi-coding-agent/tools/manage-skill";
-Bun.env.PI_PYTHON_SKIP_CHECK = "1";
-
function makeSession(
settingsOverrides: Partial> = {},
extra: Partial = {},
@@ -27,10 +25,6 @@ function makeSession(
}
describe("autolearn tool gating", () => {
- afterEach(() => {
- spyOn(os, "homedir").mockRestore();
- });
-
it("offers neither tool by default (autolearn disabled)", async () => {
const names = (await createTools(makeSession())).map(t => t.name);
expect(names).not.toContain("learn");
@@ -72,6 +66,25 @@ describe("autolearn tool gating", () => {
expect(noBackend).not.toContain("learn");
});
+ it("excludes the tools from a subagent even with an explicit list", async () => {
+ // taskDepth > 0: the controller never runs here, so a subagent's explicit
+ // whitelist must not be silently widened with write-capable tools.
+ const sub = (
+ await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "mnemopi" }, { taskDepth: 1 }), [
+ "read",
+ ])
+ ).map(t => t.name);
+ expect(sub).not.toContain("manage_skill");
+ expect(sub).not.toContain("learn");
+
+ // Nor via discovery (no explicit list) at depth.
+ const subDiscovered = (
+ await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "mnemopi" }, { taskDepth: 1 }))
+ ).map(t => t.name);
+ expect(subDiscovered).not.toContain("manage_skill");
+ expect(subDiscovered).not.toContain("learn");
+ });
+
it("offers learn with the file-based local backend", async () => {
const names = (await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "local" }))).map(
t => t.name,
From 16bad4b47ee7bd26a1d02b4f47108bf50463e177 Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 12:36:31 +0900
Subject: [PATCH 04/14] fix(coding-agent): refuse symlinked managed-skill write
targets
`writeManagedSkill` wrote `//SKILL.md` via `Bun.write`,
which follows symlinks. A planted symlink at the `` directory (or the
`SKILL.md` itself) could redirect the write outside the isolated managed root
and clobber a user-authored skill, breaking the isolation guarantee. lstat the
parent directory and the file and reject symlinks before writing; the existence
check now derives from the same lstat instead of a separate `Bun.file().exists()`.
Addresses review thread on PR #2542 (thread 11).
---
.../src/autolearn/managed-skills.ts | 27 +++++++++++++++++--
.../test/autolearn-managed-skills.test.ts | 18 +++++++++++++
2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/packages/coding-agent/src/autolearn/managed-skills.ts b/packages/coding-agent/src/autolearn/managed-skills.ts
index 4f55aef28..7d65d7fcb 100644
--- a/packages/coding-agent/src/autolearn/managed-skills.ts
+++ b/packages/coding-agent/src/autolearn/managed-skills.ts
@@ -101,8 +101,31 @@ export async function writeManagedSkill(input: WriteManagedSkillInput): Promise<
`Managed skill is ${bytes} bytes; the limit is ${MAX_MANAGED_SKILL_BYTES}. Trim the body or description.`,
);
}
- const file = path.join(getManagedSkillsDir(), name, "SKILL.md");
- const exists = await Bun.file(file).exists();
+ const dir = path.join(getManagedSkillsDir(), name);
+ const file = path.join(dir, "SKILL.md");
+ // Reject a symlinked skill directory or SKILL.md: Bun.write follows symlinks,
+ // so a planted link could escape the isolated managed root and clobber a file
+ // elsewhere (e.g. a user-authored skill). lstat does not follow the final
+ // component; an intermediate symlinked `dir` is caught before the file check.
+ const dirStat = await fs.lstat(dir).catch(err => {
+ if (isEnoent(err)) return null;
+ throw err;
+ });
+ if (dirStat?.isSymbolicLink()) {
+ throw new Error(
+ `Managed skill "${name}" resolves through a symlink; refusing to write outside the managed directory.`,
+ );
+ }
+ const fileStat = dirStat
+ ? await fs.lstat(file).catch(err => {
+ if (isEnoent(err)) return null;
+ throw err;
+ })
+ : null;
+ if (fileStat?.isSymbolicLink()) {
+ throw new Error(`Managed skill "${name}" SKILL.md is a symlink; refusing to overwrite it.`);
+ }
+ const exists = fileStat !== null;
if (input.action === "create" && exists) {
throw new Error(`Managed skill "${name}" already exists. Use action "update" to change it.`);
}
diff --git a/packages/coding-agent/test/autolearn-managed-skills.test.ts b/packages/coding-agent/test/autolearn-managed-skills.test.ts
index 21f29d17e..64b038681 100644
--- a/packages/coding-agent/test/autolearn-managed-skills.test.ts
+++ b/packages/coding-agent/test/autolearn-managed-skills.test.ts
@@ -122,6 +122,24 @@ describe("managed-skills primitives", () => {
const authoredEvil = path.join(tempHome, ".omp", "agent", "skills", "evil", "SKILL.md");
expect(await Bun.file(authoredEvil).exists()).toBe(false);
});
+
+ it("refuses to write through a symlinked skill directory", async () => {
+ const managedRoot = getManagedSkillsDir();
+ await fs.mkdir(managedRoot, { recursive: true });
+ // Plant a symlink where the skill dir would live, pointing outside the
+ // isolated managed root; Bun.write would otherwise follow it.
+ const outside = await fs.mkdtemp(path.join(os.tmpdir(), "omp-escape-"));
+ try {
+ await fs.symlink(outside, path.join(managedRoot, "evil"));
+ await expect(
+ writeManagedSkill({ action: "create", name: "evil", description: "d", body: "b" }),
+ ).rejects.toThrow(/symlink/);
+ // Nothing was written through the link.
+ expect(await Bun.file(path.join(outside, "SKILL.md")).exists()).toBe(false);
+ } finally {
+ await fs.rm(outside, { recursive: true, force: true });
+ }
+ });
});
describe("deleteManagedSkill", () => {
From 3047e500e6e383ae0d9855aee2c807b5a417fd47 Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 12:36:32 +0900
Subject: [PATCH 05/14] fix(coding-agent): install auto-learn controller before
memory startup
The `AutoLearnController` was constructed inside the fire-and-forget
`startMemoryStartupTask`, after `await memoryBackend.start()`. With a slow
backend (Mnemopi/Hindsight) a fast first turn could begin and finish before the
event listener subscribed, dropping that turn's tool events so no nudge fired.
Install the controller synchronously, before the backend task, so the listener
is registered before any turn can run.
Addresses review thread on PR #2542 (thread 10).
---
packages/coding-agent/src/sdk.ts | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts
index 915f6562e..d438415eb 100644
--- a/packages/coding-agent/src/sdk.ts
+++ b/packages/coding-agent/src/sdk.ts
@@ -2658,6 +2658,16 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
}
}
+ // Install the auto-learn post-stop nudge controller synchronously, before
+ // the possibly-slow memory backend starts, once, for top-level sessions
+ // only. A fast first turn must not finish before the listener subscribes,
+ // or its tool events are missed (slower backends like Mnemopi/Hindsight
+ // widen that window). The subscription lives for the session's lifetime;
+ // the reference is intentionally discarded (the listener retains it).
+ if (settings.get("autolearn.enabled") && taskDepth === 0) {
+ new AutoLearnController({ session, settings });
+ }
+
logger.time("startMemoryStartupTask", async () => {
const memoryBackend = await resolveMemoryBackend(settings);
await memoryBackend.start({
@@ -2669,13 +2679,6 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
parentHindsightSessionState: options.parentHindsightSessionState,
parentMnemopiSessionState: options.parentMnemopiSessionState,
});
-
- // Install the auto-learn post-stop nudge controller once, for top-level
- // sessions only. The subscription lives for the session's lifetime; the
- // reference is intentionally discarded (the listener retains it).
- if (settings.get("autolearn.enabled") && taskDepth === 0) {
- new AutoLearnController({ session, settings });
- }
});
// Wire MCP manager callbacks to session for reactive tool updates.
From c84c9d40869310945aed1553d20c2333f6738f3f Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 12:36:32 +0900
Subject: [PATCH 06/14] fix(coding-agent): harden auto-learn suppression and
skip goal mode
Two controller bugs from review:
- The post-stop nudge still queued a passive `nextTurn` message during goal
mode (goal mode only disabled `autoContinue`). That message rides the goal
continuation and can divert the goal loop into capture. Return early when
goal mode is active.
- `#suppressNext` was latched before the fire-and-forget `sendCustomMessage`.
It must arm synchronously (the synthetic turn's `agent_end` fires inside
`sendCustomMessage` before it resolves), but a rejected or *deferred* dispatch
(ACP clients downgrade `triggerTurn` to a queue) then produces no `agent_end`,
so the latch swallowed the next real stop. `sendCustomMessage` now returns
whether it actually started a turn; the controller disarms the latch when no
turn ran (rejection or deferral).
`task/executor.ts` widens its pending-message array to `Promise[]` to
absorb the new return type (the resolved values are discarded).
Addresses review threads on PR #2542 (threads 5, 9, 12).
---
.../coding-agent/src/autolearn/controller.ts | 23 +++++++--
.../coding-agent/src/session/agent-session.ts | 23 ++++++---
packages/coding-agent/src/task/executor.ts | 2 +-
.../test/autolearn-controller.test.ts | 50 ++++++++++++++++---
4 files changed, 78 insertions(+), 20 deletions(-)
diff --git a/packages/coding-agent/src/autolearn/controller.ts b/packages/coding-agent/src/autolearn/controller.ts
index 07c3d6ba2..ee69034e5 100644
--- a/packages/coding-agent/src/autolearn/controller.ts
+++ b/packages/coding-agent/src/autolearn/controller.ts
@@ -82,11 +82,18 @@ export class AutoLearnController {
if (toolCalls < minToolCalls) return;
// Never interrupt plan-mode review.
if (this.#session.getPlanModeState()?.enabled) return;
+ // Never divert an active goal loop: a passive nudge would ride the goal
+ // continuation, and auto-continue would compete with it. Skip entirely.
+ if (this.#session.getGoalModeState()?.enabled) return;
- // Auto-run a capture turn only when explicitly enabled and not competing
- // with a goal-mode continuation; otherwise ride the next turn passively.
- const autoContinue =
- this.#settings.get("autolearn.autoContinue") === true && !this.#session.getGoalModeState()?.enabled;
+ // Auto-run a capture turn only when explicitly enabled; otherwise the
+ // hidden reminder rides the next real turn passively.
+ const autoContinue = this.#settings.get("autolearn.autoContinue") === true;
+ // Arm suppression synchronously: the synthetic capture turn's agent_end
+ // fires inside sendCustomMessage (before it resolves), so the flag must be
+ // set before then. Disarm when no turn actually started — a deferred/queued
+ // dispatch or a failed send produces no agent_end, and a latched flag would
+ // otherwise swallow the next real stop.
if (autoContinue) this.#suppressNext = true;
this.#session
@@ -99,6 +106,12 @@ export class AutoLearnController {
},
{ deliverAs: "nextTurn", triggerTurn: autoContinue },
)
- .catch(err => logger.warn("auto-learn nudge delivery failed", { err }));
+ .then(started => {
+ if (!started) this.#suppressNext = false;
+ })
+ .catch(err => {
+ this.#suppressNext = false;
+ logger.warn("auto-learn nudge delivery failed", { err });
+ });
}
}
diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts
index c53d6cc16..c69741096 100644
--- a/packages/coding-agent/src/session/agent-session.ts
+++ b/packages/coding-agent/src/session/agent-session.ts
@@ -5201,11 +5201,17 @@ export class AgentSession {
* - Streaming: queue as steer/follow-up or store for next turn
* - Not streaming + triggerTurn: appends to state/session, starts new turn unless the client cannot own it
* - Not streaming + no trigger: appends to state/session, no turn
+ *
+ * @returns true iff this call synchronously started a new turn (awaited
+ * `agent.prompt`); false when the message was queued/appended without a turn
+ * — including when `triggerTurn` is downgraded because the client defers
+ * agent-initiated turns. Callers that must mirror the resulting `agent_end`
+ * use this to avoid acting on a turn that never ran.
*/
async sendCustomMessage(
message: Pick, "customType" | "content" | "display" | "details" | "attribution">,
options?: { triggerTurn?: boolean; deliverAs?: "steer" | "followUp" | "nextTurn"; queueChipText?: string },
- ): Promise {
+ ): Promise {
const details =
options?.queueChipText && options.deliverAs !== "nextTurn"
? ({
@@ -5229,7 +5235,7 @@ export class AgentSession {
if (this.isStreaming) {
if (options?.deliverAs === "nextTurn") {
this.#queueHiddenNextTurnMessage(normalizedAppMessage, options?.triggerTurn ?? false);
- return;
+ return false;
}
if (options?.deliverAs === "followUp") {
@@ -5238,17 +5244,17 @@ export class AgentSession {
this.agent.steer(normalizedAppMessage);
}
this.#scheduleIdleQueueDrain();
- return;
+ return false;
}
if (options?.deliverAs === "nextTurn") {
if (options?.triggerTurn) {
if (this.#clientBridge?.deferAgentInitiatedTurns && !this.#allowAcpAgentInitiatedTurns) {
this.#queueHiddenNextTurnMessage(normalizedAppMessage, false);
- return;
+ return false;
}
await this.agent.prompt(normalizedAppMessage);
- return;
+ return true;
}
this.agent.appendMessage(normalizedAppMessage);
this.sessionManager.appendCustomMessageEntry(
@@ -5258,16 +5264,16 @@ export class AgentSession {
message.details,
message.attribution ?? "agent",
);
- return;
+ return false;
}
if (options?.triggerTurn) {
if (this.#clientBridge?.deferAgentInitiatedTurns && !this.#allowAcpAgentInitiatedTurns) {
this.#queueHiddenNextTurnMessage(normalizedAppMessage, false);
- return;
+ return false;
}
await this.agent.prompt(normalizedAppMessage);
- return;
+ return true;
}
this.agent.appendMessage(normalizedAppMessage);
@@ -5278,6 +5284,7 @@ export class AgentSession {
message.details,
message.attribution ?? "agent",
);
+ return false;
}
/**
diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts
index 8da186151..5d7f4b93d 100644
--- a/packages/coding-agent/src/task/executor.ts
+++ b/packages/coding-agent/src/task/executor.ts
@@ -2005,7 +2005,7 @@ export async function runSubprocess(options: ExecutorOptions): Promise[] = [];
+ const pendingExtensionMessages: Promise[] = [];
if (extensionRunner) {
extensionRunner.initialize(
{
diff --git a/packages/coding-agent/test/autolearn-controller.test.ts b/packages/coding-agent/test/autolearn-controller.test.ts
index 4e3a75c3f..4556e6efd 100644
--- a/packages/coding-agent/test/autolearn-controller.test.ts
+++ b/packages/coding-agent/test/autolearn-controller.test.ts
@@ -13,14 +13,21 @@ class FakeSession {
readonly sent: CapturedNudge[] = [];
planEnabled = false;
goalEnabled = false;
+ /** Whether a triggerTurn dispatch actually starts a synthetic turn. */
+ turnStarts = true;
+ /** Force the dispatch to reject (models a failed send). */
+ failSend = false;
subscribe(listener: (event: AgentSessionEvent) => void): () => void {
this.listeners.push(listener);
return () => {};
}
- async sendCustomMessage(message: CapturedNudge["message"], options?: CapturedNudge["options"]): Promise {
+ async sendCustomMessage(message: CapturedNudge["message"], options?: CapturedNudge["options"]): Promise {
+ if (this.failSend) throw new Error("send failed");
this.sent.push({ message, options });
+ // Mirror AgentSession: a turn starts only when triggerTurn is honored.
+ return options?.triggerTurn === true && this.turnStarts;
}
getPlanModeState(): { enabled: boolean } | undefined {
@@ -127,18 +134,19 @@ describe("AutoLearnController", () => {
expect(session.sent).toHaveLength(1);
});
- it("downgrades autoContinue to a passive nudge during goal mode", () => {
+ it("does not nudge during goal mode and leaks no suppression latch", () => {
const session = new FakeSession();
session.goalEnabled = true;
install(session, { "autolearn.autoContinue": true });
session.toolCalls(5);
session.agentEnd();
- expect(session.sent).toHaveLength(1);
- expect(session.sent[0]?.options?.triggerTurn).toBe(false);
- // Passive => no suppression; the next qualifying stop fires again.
+ // Goal mode owns the continuation; auto-learn stays out of the loop.
+ expect(session.sent).toHaveLength(0);
+ // The skipped stop must not arm suppression for the next non-goal stop.
+ session.goalEnabled = false;
session.toolCalls(5);
session.agentEnd();
- expect(session.sent).toHaveLength(2);
+ expect(session.sent).toHaveLength(1);
});
it("auto-runs a capture turn and suppresses exactly one follow-up agent_end", () => {
@@ -161,6 +169,36 @@ describe("AutoLearnController", () => {
expect(session.sent).toHaveLength(2);
});
+ it("disarms suppression when the capture turn is deferred (not started)", async () => {
+ const session = new FakeSession();
+ // triggerTurn honored but downgraded to a queue: no synthetic agent_end.
+ session.turnStarts = false;
+ install(session, { "autolearn.autoContinue": true });
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(1);
+ expect(session.sent[0]?.options?.triggerTurn).toBe(true);
+ await Bun.sleep(1); // flush the async disarm
+ // No turn ran, so the next real stop must still nudge.
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(2);
+ });
+
+ it("disarms suppression when the capture-turn dispatch fails", async () => {
+ const session = new FakeSession();
+ session.failSend = true;
+ install(session, { "autolearn.autoContinue": true });
+ session.toolCalls(5);
+ session.agentEnd(); // dispatch rejects: armed, then disarmed in .catch
+ expect(session.sent).toHaveLength(0);
+ await Bun.sleep(1); // flush the async disarm
+ session.failSend = false;
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(1);
+ });
+
it("respects a custom minToolCalls threshold", () => {
const session = new FakeSession();
install(session, { "autolearn.minToolCalls": 2 });
From 62c3d504984bee6626df959364fd0bfc1e572b48 Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 12:51:57 +0900
Subject: [PATCH 07/14] fix(coding-agent): always install auto-learn controller
for top-level sessions
The controller was only constructed when `autolearn.enabled` was true at
`createAgentSession` time. Because `newSession` reuses the session without
rerunning startup, enabling the setting via the UI mid-session never installed
the listener, so the feature stayed inert until the app was recreated. The
controller already re-checks the live flag at fire time, so install it for every
top-level session and let that check gate activation; when disabled it only
counts tool calls and returns.
Addresses review thread on PR #2542 (thread 18).
---
packages/coding-agent/src/sdk.ts | 20 +++++++++++++------
.../test/autolearn-controller.test.ts | 18 +++++++++++++++++
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts
index d438415eb..0853724c1 100644
--- a/packages/coding-agent/src/sdk.ts
+++ b/packages/coding-agent/src/sdk.ts
@@ -2659,12 +2659,20 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
}
// Install the auto-learn post-stop nudge controller synchronously, before
- // the possibly-slow memory backend starts, once, for top-level sessions
- // only. A fast first turn must not finish before the listener subscribes,
- // or its tool events are missed (slower backends like Mnemopi/Hindsight
- // widen that window). The subscription lives for the session's lifetime;
- // the reference is intentionally discarded (the listener retains it).
- if (settings.get("autolearn.enabled") && taskDepth === 0) {
+ // the possibly-slow memory backend starts, once per top-level session. A
+ // fast first turn must not finish before the listener subscribes, or its
+ // tool events are missed (slower backends like Mnemopi/Hindsight widen that
+ // window).
+ //
+ // Installed regardless of the current `autolearn.enabled` value: the
+ // controller re-checks the live flag at fire time (`#onAgentEnd`), and
+ // `newSession` reuses the session without rerunning startup, so gating the
+ // install on the enable-time flag would mean toggling the setting on
+ // mid-session never activates until the app is recreated. When disabled the
+ // controller only counts tool calls and returns — negligible cost. The
+ // subscription lives for the session's lifetime; the reference is
+ // intentionally discarded (the listener retains it).
+ if (taskDepth === 0) {
new AutoLearnController({ session, settings });
}
diff --git a/packages/coding-agent/test/autolearn-controller.test.ts b/packages/coding-agent/test/autolearn-controller.test.ts
index 4556e6efd..2e201a166 100644
--- a/packages/coding-agent/test/autolearn-controller.test.ts
+++ b/packages/coding-agent/test/autolearn-controller.test.ts
@@ -134,6 +134,24 @@ describe("AutoLearnController", () => {
expect(session.sent).toHaveLength(1);
});
+ it("activates when autolearn is enabled mid-session (installed while disabled)", () => {
+ const session = new FakeSession();
+ // The controller install is now unconditional for top-level sessions, so it
+ // can be constructed while the feature is OFF. Toggling it on mid-session
+ // must take effect without recreating the session (the fire-time re-check
+ // handles activation).
+ const settings = Settings.isolated({});
+ settings.set("autolearn.enabled", false);
+ new AutoLearnController({ session: session as unknown as AgentSession, settings });
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(0); // disabled at install: no nudge
+ settings.set("autolearn.enabled", true);
+ session.toolCalls(5);
+ session.agentEnd();
+ expect(session.sent).toHaveLength(1); // enabled mid-session: now fires
+ });
+
it("does not nudge during goal mode and leaks no suppression latch", () => {
const session = new FakeSession();
session.goalEnabled = true;
From 449e0e68038d984e9ce490d5a181f1f639350c9d Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 12:51:58 +0900
Subject: [PATCH 08/14] fix(coding-agent): create managed skills atomically
with O_EXCL
`writeManagedSkill` resolved create-vs-update with a check-then-write, so two
concurrent creates for the same name could both pass the existence check and the
second would stomp the first. Create now uses an `O_CREAT|O_EXCL` ("wx") write
that fails atomically if the file exists (closing the race) and refuses to
follow a symlinked `SKILL.md`. Update keeps the lstat existence + symlink guard.
Addresses review thread on PR #2542 (thread 6).
---
.../src/autolearn/managed-skills.ts | 43 +++++++++++--------
1 file changed, 26 insertions(+), 17 deletions(-)
diff --git a/packages/coding-agent/src/autolearn/managed-skills.ts b/packages/coding-agent/src/autolearn/managed-skills.ts
index 7d65d7fcb..202a617e7 100644
--- a/packages/coding-agent/src/autolearn/managed-skills.ts
+++ b/packages/coding-agent/src/autolearn/managed-skills.ts
@@ -103,10 +103,9 @@ export async function writeManagedSkill(input: WriteManagedSkillInput): Promise<
}
const dir = path.join(getManagedSkillsDir(), name);
const file = path.join(dir, "SKILL.md");
- // Reject a symlinked skill directory or SKILL.md: Bun.write follows symlinks,
- // so a planted link could escape the isolated managed root and clobber a file
- // elsewhere (e.g. a user-authored skill). lstat does not follow the final
- // component; an intermediate symlinked `dir` is caught before the file check.
+ // Reject a symlinked skill directory: an intermediate symlink would let the
+ // write escape the isolated managed root (e.g. clobber a user-authored skill).
+ // lstat does not follow the final component, so a symlinked `dir` is caught here.
const dirStat = await fs.lstat(dir).catch(err => {
if (isEnoent(err)) return null;
throw err;
@@ -116,22 +115,32 @@ export async function writeManagedSkill(input: WriteManagedSkillInput): Promise<
`Managed skill "${name}" resolves through a symlink; refusing to write outside the managed directory.`,
);
}
- const fileStat = dirStat
- ? await fs.lstat(file).catch(err => {
- if (isEnoent(err)) return null;
- throw err;
- })
- : null;
- if (fileStat?.isSymbolicLink()) {
- throw new Error(`Managed skill "${name}" SKILL.md is a symlink; refusing to overwrite it.`);
+ if (input.action === "create") {
+ await fs.mkdir(dir, { recursive: true });
+ // O_CREAT|O_EXCL ("wx"): atomic create that fails if the file already
+ // exists — closing the check-then-write race two concurrent creates would
+ // otherwise lose — and refuses to follow a symlinked SKILL.md (EEXIST).
+ try {
+ await fs.writeFile(file, content, { flag: "wx" });
+ } catch (err) {
+ if ((err as { code?: string }).code === "EEXIST") {
+ throw new Error(`Managed skill "${name}" already exists. Use action "update" to change it.`);
+ }
+ throw err;
+ }
+ return { path: file };
}
- const exists = fileStat !== null;
- if (input.action === "create" && exists) {
- throw new Error(`Managed skill "${name}" already exists. Use action "update" to change it.`);
- }
- if (input.action === "update" && !exists) {
+ // update: the file must already exist and must not be a symlink.
+ const fileStat = await fs.lstat(file).catch(err => {
+ if (isEnoent(err)) return null;
+ throw err;
+ });
+ if (fileStat === null) {
throw new Error(`Managed skill "${name}" does not exist. Use action "create" to add it.`);
}
+ if (fileStat.isSymbolicLink()) {
+ throw new Error(`Managed skill "${name}" SKILL.md is a symlink; refusing to overwrite it.`);
+ }
await Bun.write(file, content);
return { path: file };
}
From f83084c92c0d18db796419624bce6e9893a47cda Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 12:51:58 +0900
Subject: [PATCH 09/14] docs(coding-agent): pin load-bearing custom-dir vs
managed skill ordering
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The managed-skill discovery block vetoes a name already in `skillMap` to let a
custom-directory authored skill win. Custom dirs never enter `result.all`, so
they are absent from `enabledAuthoredNames` — the map check is the only veto, and
it only works because the custom-dir loop runs first. Add a comment so a future
refactor doesn't invert the order.
Addresses review thread on PR #2542 (thread 7).
---
packages/coding-agent/src/extensibility/skills.ts | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/packages/coding-agent/src/extensibility/skills.ts b/packages/coding-agent/src/extensibility/skills.ts
index 5724e469e..53043b9c0 100644
--- a/packages/coding-agent/src/extensibility/skills.ts
+++ b/packages/coding-agent/src/extensibility/skills.ts
@@ -336,7 +336,12 @@ export async function loadSkills(options: LoadSkillsOptions = {}): Promise
Date: Sun, 14 Jun 2026 12:51:58 +0900
Subject: [PATCH 10/14] fix(coding-agent): clamp learned-lessons injection
budget to non-negative
`truncateByApproxTokens` appends a truncation marker, so a truncated memory
summary can exceed `limit * 4` chars and drive the remaining lessons budget
negative. Clamp to 0 and skip lessons entirely when the summary already fills the
budget, instead of feeding a negative limit to the truncator.
Addresses review thread on PR #2542 (thread 15).
---
packages/coding-agent/src/memories/index.ts | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/packages/coding-agent/src/memories/index.ts b/packages/coding-agent/src/memories/index.ts
index eabcc28cb..9532b5ff7 100644
--- a/packages/coding-agent/src/memories/index.ts
+++ b/packages/coding-agent/src/memories/index.ts
@@ -172,8 +172,11 @@ export async function buildMemoryToolDeveloperInstructions(
// Lessons share ONE injection budget with the summary so the combined block
// stays within `summaryInjectionTokenLimit` (~4 chars/token, matching
// truncateByApproxTokens). With no summary, lessons get the whole budget.
- const learnedBudget = cfg.summaryInjectionTokenLimit - Math.ceil(summaryOut.length / 4);
- const learnedOut = learned ? truncateByApproxTokens(learned, learnedBudget).trim() : "";
+ // Clamp to 0: truncateByApproxTokens appends a marker, so a truncated summary
+ // can exceed `limit * 4` chars and drive the remainder negative — when the
+ // summary already fills the budget, lessons are simply dropped.
+ const learnedBudget = Math.max(0, cfg.summaryInjectionTokenLimit - Math.ceil(summaryOut.length / 4));
+ const learnedOut = learned && learnedBudget > 0 ? truncateByApproxTokens(learned, learnedBudget).trim() : "";
if (!summaryOut && !learnedOut) return undefined;
return prompt.render(readPathTemplate, {
From 66da2aabef8275c15a8c51ee9f42a39d8472f525 Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 12:51:59 +0900
Subject: [PATCH 11/14] fix(coding-agent): tighten learn/manage_skill input
contracts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
- learn (mnemopi): `rememberScoped` returns undefined when the retain failed
(closed DB / disk error). The tool ignored it and reported "Lesson stored"
(and could still mint a skill), silently losing the lesson. Mirror
`mnemopiBackend.save` and fail loudly when no id is returned.
- manage_skill: enforce the action/field contract in the schema via a cross-field
refine (create/update require description+body; delete needs only name) instead
of relying solely on a runtime throw in execute. Kept as a refine, not a
discriminated union, so the wire schema stays a single root object — both
strict structured-output mode and the Anthropic tool-schema builder require
that.
Addresses review threads on PR #2542 (threads 19, 14).
---
packages/coding-agent/src/tools/learn.ts | 8 ++++-
.../coding-agent/src/tools/manage-skill.ts | 33 ++++++++++++++-----
.../test/autolearn-tools-gating.test.ts | 28 ++++++++++++++++
3 files changed, 59 insertions(+), 10 deletions(-)
diff --git a/packages/coding-agent/src/tools/learn.ts b/packages/coding-agent/src/tools/learn.ts
index c9197660f..7faabe26f 100644
--- a/packages/coding-agent/src/tools/learn.ts
+++ b/packages/coding-agent/src/tools/learn.ts
@@ -58,7 +58,7 @@ export class LearnTool implements AgentTool {
if (!state) {
throw new Error("Mnemopi backend is not initialised for this session.");
}
- state.rememberScoped(params.memory, {
+ const id = state.rememberScoped(params.memory, {
source: "coding-agent-learn",
importance: 0.8,
metadata: {
@@ -73,6 +73,12 @@ export class LearnTool implements AgentTool {
veracity: "tool",
memoryType: "fact",
});
+ // rememberScoped returns undefined when the retain failed (closed DB /
+ // disk error); mirror mnemopiBackend.save and fail loudly rather than
+ // reporting (and minting a skill for) a lesson that was silently dropped.
+ if (!id) {
+ throw new Error("Mnemopi did not store the lesson (no memory id returned).");
+ }
} else if (backend === "local") {
const result = await localBackend.save?.(
{ agentDir: this.session.settings.getAgentDir(), cwd: this.session.settings.getCwd() },
diff --git a/packages/coding-agent/src/tools/manage-skill.ts b/packages/coding-agent/src/tools/manage-skill.ts
index f82fa802a..12f7c8e12 100644
--- a/packages/coding-agent/src/tools/manage-skill.ts
+++ b/packages/coding-agent/src/tools/manage-skill.ts
@@ -5,15 +5,27 @@ import { deleteManagedSkill, getManagedSkillsDir, writeManagedSkill } from "../a
import manageSkillDescription from "../prompts/tools/manage-skill.md" with { type: "text" };
import type { ToolSession } from ".";
-const manageSkillSchema = z.object({
- action: z.enum(["create", "update", "delete"]),
- name: z.string().describe("kebab-case skill name"),
- description: z
- .string()
- .describe("one-line description of when to use the skill (required for create/update)")
- .optional(),
- body: z.string().describe("the SKILL.md body in markdown, no frontmatter (required for create/update)").optional(),
-});
+const manageSkillSchema = z
+ .object({
+ action: z.enum(["create", "update", "delete"]),
+ name: z.string().describe("kebab-case skill name"),
+ description: z
+ .string()
+ .describe("one-line description of when to use the skill (required for create/update)")
+ .optional(),
+ body: z
+ .string()
+ .describe("the SKILL.md body in markdown, no frontmatter (required for create/update)")
+ .optional(),
+ })
+ // Enforce the action/field contract at validation time rather than only in
+ // execute. Kept as a cross-field refine (not a discriminated union) so the
+ // wire schema stays a single root object — strict structured-output mode and
+ // the Anthropic tool-schema builder both require that.
+ .refine(p => p.action === "delete" || (p.description !== undefined && p.body !== undefined), {
+ message: '"create" and "update" require both "description" and "body".',
+ path: ["description"],
+ });
export type ManageSkillParams = z.infer;
@@ -47,6 +59,9 @@ export class ManageSkillTool implements AgentTool {
};
}
+ // Defensive narrowing: the schema refine already rejects create/update
+ // without both fields, so this is unreachable for valid input — it only
+ // proves the strings are present to `writeManagedSkill`'s typed contract.
if (!params.description || !params.body) {
throw new Error(`"${params.action}" requires both "description" and "body".`);
}
diff --git a/packages/coding-agent/test/autolearn-tools-gating.test.ts b/packages/coding-agent/test/autolearn-tools-gating.test.ts
index ab98f7288..90a5641f6 100644
--- a/packages/coding-agent/test/autolearn-tools-gating.test.ts
+++ b/packages/coding-agent/test/autolearn-tools-gating.test.ts
@@ -130,6 +130,14 @@ describe("manage_skill execute", () => {
);
await expect(tool().execute("4", { action: "delete", name: "absent" })).rejects.toThrow(/does not exist/);
});
+
+ it("schema rejects create/update without description+body but allows delete", () => {
+ const schema = tool().parameters;
+ expect(schema.safeParse({ action: "create", name: "x" }).success).toBe(false);
+ expect(schema.safeParse({ action: "update", name: "x", description: "d" }).success).toBe(false);
+ expect(schema.safeParse({ action: "create", name: "x", description: "d", body: "b" }).success).toBe(true);
+ expect(schema.safeParse({ action: "delete", name: "x" }).success).toBe(true);
+ });
});
describe("learn execute", () => {
@@ -188,4 +196,24 @@ describe("learn execute", () => {
// The memory half still ran.
expect(remembered).toHaveLength(1);
});
+
+ it("fails the lesson and skips the skill when mnemopi returns no id", async () => {
+ const failingState = {
+ sessionId: "sess-2",
+ session: { sessionManager: { getCwd: () => "/tmp/work" } },
+ rememberScoped: () => undefined,
+ };
+ const session = makeSession(
+ { "autolearn.enabled": true, "memory.backend": "mnemopi" },
+ { getMnemopiSessionState: () => failingState as unknown as MnemopiSessionState },
+ );
+ await expect(
+ new LearnTool(session).execute("5", {
+ memory: "lesson",
+ skill: { action: "create", name: "should-not-exist", description: "d", body: "b" },
+ }),
+ ).rejects.toThrow(/did not store/i);
+ // A failed lesson must not leave a minted skill behind.
+ expect(await Bun.file(path.join(getManagedSkillsDir(), "should-not-exist", "SKILL.md")).exists()).toBe(false);
+ });
});
From 12c4078b4bce44d2e8678a7cf4c7c334251b3145 Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 14:00:19 +0900
Subject: [PATCH 12/14] fix(coding-agent): harden managed-skill writes (symlink
root, empty content, races)
Grill findings on the auto-learn change-set:
- Reject a symlinked managed-skills ROOT (not just the per-skill dir): lstat on a
child follows intermediate components, so a symlinked root would let an
otherwise-valid name write/delete outside the isolated directory. Added
`assertManagedRootSafe()` to create, update, and delete.
- Reject empty content: an all-whitespace/control description sanitizes to "" and
the `requireDescription` discovery scan then silently drops the skill, so the
tool would report success for a skill that never appears. Reject empty
description/body before writing.
- Serialize create/update/delete on the same skill name (`serializeSkillMutation`):
both tools are non-exclusive, so a parallel batch could interleave (e.g. update
observing the file mid-delete). O_EXCL only covered duplicate-create.
Tests: symlinked root/dir/file rejection (create + update + delete), empty
description/body, concurrent create+update ordering, concurrent-create one-winner.
---
.../src/autolearn/managed-skills.ts | 154 ++++++++++++------
.../test/autolearn-managed-skills.test.ts | 83 ++++++++++
2 files changed, 190 insertions(+), 47 deletions(-)
diff --git a/packages/coding-agent/src/autolearn/managed-skills.ts b/packages/coding-agent/src/autolearn/managed-skills.ts
index 202a617e7..52c994aff 100644
--- a/packages/coding-agent/src/autolearn/managed-skills.ts
+++ b/packages/coding-agent/src/autolearn/managed-skills.ts
@@ -89,10 +89,57 @@ export interface WriteManagedSkillInput {
body: string;
}
+/**
+ * Serialize create/update/delete on the same skill name. Both tools are
+ * non-exclusive, so a parallel tool batch in one turn can run two mutations on
+ * the same skill at once (e.g. an update observing the file mid-delete). This
+ * per-name promise chain runs same-skill mutations in submission order while
+ * different names still proceed in parallel. In-process only; cross-process
+ * races are out of scope.
+ */
+const skillMutationChains = new Map>();
+function serializeSkillMutation(name: string, op: () => Promise): Promise {
+ const prev = skillMutationChains.get(name) ?? Promise.resolve();
+ const run = prev.then(op, op);
+ const guarded = run.catch(() => {});
+ skillMutationChains.set(name, guarded);
+ void guarded.finally(() => {
+ if (skillMutationChains.get(name) === guarded) skillMutationChains.delete(name);
+ });
+ return run;
+}
+
+/**
+ * Reject when the managed-skills root itself is a symlink. lstat on a child
+ * follows intermediate components, so a symlinked root would let an otherwise
+ * valid name write/delete outside the isolated directory (e.g. onto authored
+ * skills). Checked before composing any child path.
+ */
+async function assertManagedRootSafe(): Promise {
+ const rootStat = await fs.lstat(getManagedSkillsDir()).catch(err => {
+ if (isEnoent(err)) return null;
+ throw err;
+ });
+ if (rootStat?.isSymbolicLink()) {
+ throw new Error("The managed-skills root is a symlink; refusing to operate outside the managed directory.");
+ }
+}
+
/** Create or update a managed `SKILL.md`. Returns the resolved file path. */
export async function writeManagedSkill(input: WriteManagedSkillInput): Promise<{ path: string }> {
const name = sanitizeSkillName(input.name);
- const content = `${toSkillFrontmatter(name, input.description)}\n${input.body.trim()}\n`;
+ const description = sanitizeManagedDescription(input.description);
+ const body = input.body.trim();
+ // Reject empty content: an all-whitespace/control description sanitizes to ""
+ // and the `requireDescription` discovery scan then silently drops the skill,
+ // so the tool would report success for a skill that never appears.
+ if (!description) {
+ throw new Error(`Managed skill "${name}" needs a non-empty description.`);
+ }
+ if (!body) {
+ throw new Error(`Managed skill "${name}" needs a non-empty body.`);
+ }
+ const content = `${toSkillFrontmatter(name, description)}\n${body}\n`;
// Cap the UTF-8 byte size of the FINAL file (body + description + frontmatter),
// not the UTF-16 code-unit length of the body alone.
const bytes = Buffer.byteLength(content, "utf8");
@@ -101,60 +148,73 @@ export async function writeManagedSkill(input: WriteManagedSkillInput): Promise<
`Managed skill is ${bytes} bytes; the limit is ${MAX_MANAGED_SKILL_BYTES}. Trim the body or description.`,
);
}
- const dir = path.join(getManagedSkillsDir(), name);
- const file = path.join(dir, "SKILL.md");
- // Reject a symlinked skill directory: an intermediate symlink would let the
- // write escape the isolated managed root (e.g. clobber a user-authored skill).
- // lstat does not follow the final component, so a symlinked `dir` is caught here.
- const dirStat = await fs.lstat(dir).catch(err => {
- if (isEnoent(err)) return null;
- throw err;
- });
- if (dirStat?.isSymbolicLink()) {
- throw new Error(
- `Managed skill "${name}" resolves through a symlink; refusing to write outside the managed directory.`,
- );
- }
- if (input.action === "create") {
- await fs.mkdir(dir, { recursive: true });
- // O_CREAT|O_EXCL ("wx"): atomic create that fails if the file already
- // exists — closing the check-then-write race two concurrent creates would
- // otherwise lose — and refuses to follow a symlinked SKILL.md (EEXIST).
- try {
- await fs.writeFile(file, content, { flag: "wx" });
- } catch (err) {
- if ((err as { code?: string }).code === "EEXIST") {
- throw new Error(`Managed skill "${name}" already exists. Use action "update" to change it.`);
- }
+ return serializeSkillMutation(name, async () => {
+ await assertManagedRootSafe();
+ const dir = path.join(getManagedSkillsDir(), name);
+ const file = path.join(dir, "SKILL.md");
+ // Reject a symlinked skill directory: an intermediate symlink would let the
+ // write escape the isolated managed root. lstat does not follow the final
+ // component, so a symlinked `dir` is caught here.
+ const dirStat = await fs.lstat(dir).catch(err => {
+ if (isEnoent(err)) return null;
throw err;
+ });
+ if (dirStat?.isSymbolicLink()) {
+ throw new Error(
+ `Managed skill "${name}" resolves through a symlink; refusing to write outside the managed directory.`,
+ );
}
+ if (input.action === "create") {
+ await fs.mkdir(dir, { recursive: true });
+ // O_CREAT|O_EXCL ("wx"): atomic create that fails if the file already
+ // exists (closing the check-then-write race) and refuses a symlinked SKILL.md.
+ try {
+ await fs.writeFile(file, content, { flag: "wx" });
+ } catch (err) {
+ if ((err as { code?: string }).code === "EEXIST") {
+ throw new Error(`Managed skill "${name}" already exists. Use action "update" to change it.`);
+ }
+ throw err;
+ }
+ return { path: file };
+ }
+ // update: the file must already exist and must not be a symlink.
+ const fileStat = await fs.lstat(file).catch(err => {
+ if (isEnoent(err)) return null;
+ throw err;
+ });
+ if (fileStat === null) {
+ throw new Error(`Managed skill "${name}" does not exist. Use action "create" to add it.`);
+ }
+ if (fileStat.isSymbolicLink()) {
+ throw new Error(`Managed skill "${name}" SKILL.md is a symlink; refusing to overwrite it.`);
+ }
+ await Bun.write(file, content);
return { path: file };
- }
- // update: the file must already exist and must not be a symlink.
- const fileStat = await fs.lstat(file).catch(err => {
- if (isEnoent(err)) return null;
- throw err;
});
- if (fileStat === null) {
- throw new Error(`Managed skill "${name}" does not exist. Use action "create" to add it.`);
- }
- if (fileStat.isSymbolicLink()) {
- throw new Error(`Managed skill "${name}" SKILL.md is a symlink; refusing to overwrite it.`);
- }
- await Bun.write(file, content);
- return { path: file };
}
/** Delete a managed skill directory. Throws when it does not exist. */
export async function deleteManagedSkill(name: string): Promise {
const safe = sanitizeSkillName(name);
- const dir = path.join(getManagedSkillsDir(), safe);
- try {
- await fs.rm(dir, { recursive: true });
- } catch (err) {
- if (isEnoent(err)) {
- throw new Error(`Managed skill "${safe}" does not exist.`);
+ await serializeSkillMutation(safe, async () => {
+ await assertManagedRootSafe();
+ const dir = path.join(getManagedSkillsDir(), safe);
+ // Refuse to follow a symlinked skill directory (rm would delete the target).
+ const dirStat = await fs.lstat(dir).catch(err => {
+ if (isEnoent(err)) return null;
+ throw err;
+ });
+ if (dirStat?.isSymbolicLink()) {
+ throw new Error(`Managed skill "${safe}" is a symlink; refusing to delete outside the managed directory.`);
}
- throw err;
- }
+ try {
+ await fs.rm(dir, { recursive: true });
+ } catch (err) {
+ if (isEnoent(err)) {
+ throw new Error(`Managed skill "${safe}" does not exist.`);
+ }
+ throw err;
+ }
+ });
}
diff --git a/packages/coding-agent/test/autolearn-managed-skills.test.ts b/packages/coding-agent/test/autolearn-managed-skills.test.ts
index 64b038681..f7c44e200 100644
--- a/packages/coding-agent/test/autolearn-managed-skills.test.ts
+++ b/packages/coding-agent/test/autolearn-managed-skills.test.ts
@@ -140,6 +140,74 @@ describe("managed-skills primitives", () => {
await fs.rm(outside, { recursive: true, force: true });
}
});
+
+ it("rejects an empty or whitespace-only description", async () => {
+ await expect(
+ writeManagedSkill({ action: "create", name: "blank", description: " ", body: "# body" }),
+ ).rejects.toThrow(/non-empty description/);
+ // Nothing written, so discovery never silently drops a "successful" skill.
+ expect(await Bun.file(skillFile("blank")).exists()).toBe(false);
+ });
+
+ it("rejects an empty or whitespace-only body", async () => {
+ await expect(
+ writeManagedSkill({ action: "create", name: "nobody", description: "d", body: " \n " }),
+ ).rejects.toThrow(/non-empty body/);
+ });
+
+ it("refuses to write when the managed-skills root itself is a symlink", async () => {
+ const realRoot = await fs.mkdtemp(path.join(os.tmpdir(), "omp-realroot-"));
+ try {
+ await fs.mkdir(path.dirname(getManagedSkillsDir()), { recursive: true });
+ await fs.symlink(realRoot, getManagedSkillsDir());
+ await expect(
+ writeManagedSkill({ action: "create", name: "demo", description: "d", body: "b" }),
+ ).rejects.toThrow(/managed-skills root is a symlink/);
+ expect(await Bun.file(path.join(realRoot, "demo", "SKILL.md")).exists()).toBe(false);
+ } finally {
+ await fs.rm(realRoot, { recursive: true, force: true });
+ }
+ });
+
+ it("serializes a concurrent create+update of the same name in submission order", async () => {
+ const [createRes, updateRes] = await Promise.allSettled([
+ writeManagedSkill({ action: "create", name: "seq", description: "d", body: "v1" }),
+ writeManagedSkill({ action: "update", name: "seq", description: "d", body: "v2" }),
+ ]);
+ // Without serialization the update could observe the file missing and throw.
+ expect(createRes.status).toBe("fulfilled");
+ expect(updateRes.status).toBe("fulfilled");
+ const { body } = parseFrontmatter(await Bun.file(skillFile("seq")).text(), { source: "test" });
+ expect(body).toContain("v2");
+ });
+
+ it("lets exactly one of two concurrent creates win", async () => {
+ const results = await Promise.allSettled([
+ writeManagedSkill({ action: "create", name: "race", description: "d", body: "first" }),
+ writeManagedSkill({ action: "create", name: "race", description: "d", body: "second" }),
+ ]);
+ expect(results.filter(r => r.status === "fulfilled")).toHaveLength(1);
+ const rejected = results.filter(r => r.status === "rejected") as PromiseRejectedResult[];
+ expect(rejected).toHaveLength(1);
+ expect(String(rejected[0]?.reason)).toMatch(/already exists/);
+ });
+
+ it("refuses to update a SKILL.md that is a symlink", async () => {
+ await writeManagedSkill({ action: "create", name: "linky", description: "d", body: "real" });
+ const outside = await fs.mkdtemp(path.join(os.tmpdir(), "omp-link-"));
+ const target = path.join(outside, "target.md");
+ await Bun.write(target, "outside content");
+ try {
+ await fs.rm(skillFile("linky"));
+ await fs.symlink(target, skillFile("linky"));
+ await expect(
+ writeManagedSkill({ action: "update", name: "linky", description: "d", body: "hacked" }),
+ ).rejects.toThrow(/symlink/);
+ expect(await Bun.file(target).text()).toBe("outside content");
+ } finally {
+ await fs.rm(outside, { recursive: true, force: true });
+ }
+ });
});
describe("deleteManagedSkill", () => {
@@ -150,5 +218,20 @@ describe("managed-skills primitives", () => {
await expect(deleteManagedSkill("gone")).rejects.toThrow(/does not exist/);
});
+
+ it("refuses to delete through a symlinked skill directory", async () => {
+ const managedRoot = getManagedSkillsDir();
+ await fs.mkdir(managedRoot, { recursive: true });
+ const outside = await fs.mkdtemp(path.join(os.tmpdir(), "omp-deltarget-"));
+ await Bun.write(path.join(outside, "keep.txt"), "keep");
+ try {
+ await fs.symlink(outside, path.join(managedRoot, "linked"));
+ await expect(deleteManagedSkill("linked")).rejects.toThrow(/symlink/);
+ // The symlink target's contents are untouched.
+ expect(await Bun.file(path.join(outside, "keep.txt")).exists()).toBe(true);
+ } finally {
+ await fs.rm(outside, { recursive: true, force: true });
+ }
+ });
});
});
From 28cf4231729f57abda1111e279a32391ca6c9710 Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 14:00:19 +0900
Subject: [PATCH 13/14] fix(coding-agent): make auto-learn activation,
guidance, and controller consistent
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Grill findings on the auto-learn change-set — the controller nudge, the standing
guidance, and the actual tool availability could disagree:
- Guidance was rebuilt from live `autolearn.enabled`, so a mid-session enable (or
a subagent that filtered the tools out) injected guidance for tools the session
never built. `buildAutoLearnInstructions` now takes `{ manageSkill, learn }` and
is driven by the auto-learn BUILTINS that `createTools` actually built
(`builtInToolNames`) — provenance, so a same-named custom/extension tool can't
trigger it while auto-learn is off.
- The controller install reverted to gate on `autolearn.enabled && taskDepth === 0`:
the tool registry is built once at session start, so installing it while disabled
would nudge toward absent tools. The fire-time re-check still handles a
mid-session disable.
- Force-included auto-learn tools are now ACTIVATED for restricted top-level
sessions (mirroring the `yield` invariant), so a session with an explicit tool
whitelist actually exposes manage_skill/learn instead of building them inactive.
Tests: guidance gating by tool presence (none/manage-only/learn), restricted-session
activation via createAgentSession, and removal of the obsolete mid-session-enable
controller case.
---
.../coding-agent/src/autolearn/controller.ts | 20 +++---
packages/coding-agent/src/sdk.ts | 50 ++++++++++----
.../test/autolearn-controller.test.ts | 44 +++---------
.../test/sdk-autolearn-active-tools.test.ts | 68 +++++++++++++++++++
4 files changed, 124 insertions(+), 58 deletions(-)
create mode 100644 packages/coding-agent/test/sdk-autolearn-active-tools.test.ts
diff --git a/packages/coding-agent/src/autolearn/controller.ts b/packages/coding-agent/src/autolearn/controller.ts
index ee69034e5..d204ee259 100644
--- a/packages/coding-agent/src/autolearn/controller.ts
+++ b/packages/coding-agent/src/autolearn/controller.ts
@@ -21,16 +21,20 @@ const AUTOLEARN_NUDGE = autolearnNudge.trim();
const DEFAULT_MIN_TOOL_CALLS = 5;
/**
- * Build the standing auto-learn guidance for the system prompt, or null when
- * the feature is disabled. The `learn` addendum is included only when a memory
- * backend is live (the `learn` tool requires one — `hindsight`, `mnemopi`, or
- * the file-based `local` backend).
+ * Build the standing auto-learn guidance for the system prompt from the tools
+ * actually present in the active set, or null when `manage_skill` is absent.
+ *
+ * Driven by tool presence rather than live settings: the `learn`/`manage_skill`
+ * registry is built ONCE at session start (and only for top-level sessions), so
+ * keying the guidance on `autolearn.enabled` would let a mid-session enable — or
+ * a subagent that filtered the tools out — inject guidance pointing at tools the
+ * session never built. The `learn` addendum is included only when the `learn`
+ * tool is present (it requires a memory backend).
*/
-export function buildAutoLearnInstructions(settings: Settings): string | null {
- if (!settings.get("autolearn.enabled")) return null;
- const learnEnabled = ["hindsight", "mnemopi", "local"].includes(settings.get("memory.backend") ?? "");
+export function buildAutoLearnInstructions(available: { manageSkill: boolean; learn: boolean }): string | null {
+ if (!available.manageSkill) return null;
const parts = [autolearnGuidance.trim()];
- if (learnEnabled) parts.push(autolearnGuidanceLearn.trim());
+ if (available.learn) parts.push(autolearnGuidanceLearn.trim());
return parts.join("\n\n");
}
diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts
index 0853724c1..26fca2fc0 100644
--- a/packages/coding-agent/src/sdk.ts
+++ b/packages/coding-agent/src/sdk.ts
@@ -2121,7 +2121,16 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
// the now-connected servers' instructions, so they join the prompt for the
// rest of the session.
const serverInstructions = mcpManager?.getServerInstructions();
- const autoLearnInstructions = buildAutoLearnInstructions(settings);
+ // Drive guidance off the auto-learn BUILTINS that createTools actually built
+ // (provenance, not just an active name): `builtInToolNames` excludes a
+ // custom/extension tool that merely shares the name, and reflects the
+ // session-start build — so a subagent that filtered them out, a mid-session
+ // enable that never built them, or a same-named custom tool while auto-learn
+ // is off all get no guidance.
+ const autoLearnInstructions = buildAutoLearnInstructions({
+ manageSkill: builtInToolNames.includes("manage_skill"),
+ learn: builtInToolNames.includes("learn"),
+ });
const appendParts: string[] = [];
if (memoryInstructions) appendParts.push(memoryInstructions);
if (autoLearnInstructions) appendParts.push(autoLearnInstructions);
@@ -2190,6 +2199,20 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
) {
explicitlyRequestedToolNames.push("yield");
}
+ // Auto-learn builtins are force-included into the registry by `createTools`
+ // for enabled top-level sessions (tools/index.ts), but — like `yield` above —
+ // an explicit `toolNames` list would otherwise drop them from the ACTIVE set,
+ // leaving the nudge/guidance pointing at tools the model cannot call. Activate
+ // exactly the builtins createTools built (`builtInToolNames` — provenance, so a
+ // same-named custom/extension tool is never force-activated when auto-learn is
+ // off) to keep guidance, controller, and the active set consistent.
+ if (explicitlyRequestedToolNames) {
+ for (const name of ["manage_skill", "learn"]) {
+ if (builtInToolNames.includes(name) && !explicitlyRequestedToolNames.includes(name)) {
+ explicitlyRequestedToolNames.push(name);
+ }
+ }
+ }
const requestedToolNames = explicitlyRequestedToolNames ?? toolNamesFromRegistry;
const normalizedRequested = requestedToolNames.filter(name => toolRegistry.has(name));
const requestedToolNameSet = new Set(normalizedRequested);
@@ -2659,20 +2682,19 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
}
// Install the auto-learn post-stop nudge controller synchronously, before
- // the possibly-slow memory backend starts, once per top-level session. A
- // fast first turn must not finish before the listener subscribes, or its
- // tool events are missed (slower backends like Mnemopi/Hindsight widen that
- // window).
+ // the possibly-slow memory backend starts, once per top-level session, when
+ // the feature is enabled at session start. A fast first turn must not finish
+ // before the listener subscribes, or its tool events are missed.
//
- // Installed regardless of the current `autolearn.enabled` value: the
- // controller re-checks the live flag at fire time (`#onAgentEnd`), and
- // `newSession` reuses the session without rerunning startup, so gating the
- // install on the enable-time flag would mean toggling the setting on
- // mid-session never activates until the app is recreated. When disabled the
- // controller only counts tool calls and returns — negligible cost. The
- // subscription lives for the session's lifetime; the reference is
- // intentionally discarded (the listener retains it).
- if (taskDepth === 0) {
+ // Gated on `autolearn.enabled` to match the tools: `createTools` builds the
+ // `learn`/`manage_skill` registry ONCE at session start and no settings
+ // change rebuilds it, so installing the controller while disabled would let a
+ // mid-session enable fire a nudge pointing at tools the session never built.
+ // Activation is therefore a session-start decision for BOTH the controller
+ // and the tools; the fire-time re-check in `#onAgentEnd` still handles a
+ // mid-session DISABLE. The subscription lives for the session's lifetime; the
+ // reference is intentionally discarded (the listener retains it).
+ if (settings.get("autolearn.enabled") && taskDepth === 0) {
new AutoLearnController({ session, settings });
}
diff --git a/packages/coding-agent/test/autolearn-controller.test.ts b/packages/coding-agent/test/autolearn-controller.test.ts
index 2e201a166..ffb55eb93 100644
--- a/packages/coding-agent/test/autolearn-controller.test.ts
+++ b/packages/coding-agent/test/autolearn-controller.test.ts
@@ -134,24 +134,6 @@ describe("AutoLearnController", () => {
expect(session.sent).toHaveLength(1);
});
- it("activates when autolearn is enabled mid-session (installed while disabled)", () => {
- const session = new FakeSession();
- // The controller install is now unconditional for top-level sessions, so it
- // can be constructed while the feature is OFF. Toggling it on mid-session
- // must take effect without recreating the session (the fire-time re-check
- // handles activation).
- const settings = Settings.isolated({});
- settings.set("autolearn.enabled", false);
- new AutoLearnController({ session: session as unknown as AgentSession, settings });
- session.toolCalls(5);
- session.agentEnd();
- expect(session.sent).toHaveLength(0); // disabled at install: no nudge
- settings.set("autolearn.enabled", true);
- session.toolCalls(5);
- session.agentEnd();
- expect(session.sent).toHaveLength(1); // enabled mid-session: now fires
- });
-
it("does not nudge during goal mode and leaks no suppression latch", () => {
const session = new FakeSession();
session.goalEnabled = true;
@@ -227,30 +209,20 @@ describe("AutoLearnController", () => {
});
describe("buildAutoLearnInstructions", () => {
- it("returns null when auto-learn is disabled", () => {
- expect(buildAutoLearnInstructions(Settings.isolated({ "autolearn.enabled": false }))).toBeNull();
+ it("returns null when manage_skill is not in the active tool set", () => {
+ expect(buildAutoLearnInstructions({ manageSkill: false, learn: false })).toBeNull();
+ // learn without manage_skill still yields no guidance (manage_skill gates it).
+ expect(buildAutoLearnInstructions({ manageSkill: false, learn: true })).toBeNull();
});
- it("includes the learn addendum when a memory backend is live", () => {
- const text = buildAutoLearnInstructions(
- Settings.isolated({ "autolearn.enabled": true, "memory.backend": "mnemopi" }),
- );
+ it("includes the learn addendum when the learn tool is present", () => {
+ const text = buildAutoLearnInstructions({ manageSkill: true, learn: true });
expect(text).toContain("manage_skill");
expect(text).toContain("long-term memory");
});
- it("includes the learn addendum for the file-based local backend", () => {
- const text = buildAutoLearnInstructions(
- Settings.isolated({ "autolearn.enabled": true, "memory.backend": "local" }),
- );
- expect(text).toContain("manage_skill");
- expect(text).toContain("long-term memory");
- });
-
- it("omits the learn addendum when no memory backend is configured", () => {
- const text = buildAutoLearnInstructions(
- Settings.isolated({ "autolearn.enabled": true, "memory.backend": "off" }),
- );
+ it("omits the learn addendum when only manage_skill is present", () => {
+ const text = buildAutoLearnInstructions({ manageSkill: true, learn: false });
expect(text).toContain("manage_skill");
expect(text).not.toContain("long-term memory");
});
diff --git a/packages/coding-agent/test/sdk-autolearn-active-tools.test.ts b/packages/coding-agent/test/sdk-autolearn-active-tools.test.ts
new file mode 100644
index 000000000..298b2a505
--- /dev/null
+++ b/packages/coding-agent/test/sdk-autolearn-active-tools.test.ts
@@ -0,0 +1,68 @@
+import { afterAll, beforeAll, describe, expect, it } from "bun:test";
+import * as fs from "node:fs";
+import * as os from "node:os";
+import * as path from "node:path";
+import { AuthStorage } from "@oh-my-pi/pi-ai";
+import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
+import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
+import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
+import { createAgentSession } from "@oh-my-pi/pi-coding-agent/sdk";
+import type { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
+import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
+import { Snowflake } from "@oh-my-pi/pi-utils";
+
+// Guards the auto-learn tool ACTIVATION wiring in createAgentSession: createTools
+// force-includes manage_skill into the built registry for an enabled top-level
+// session, but an explicit `toolNames` whitelist would otherwise drop it from the
+// active set — so the SDK must re-activate it (mirroring the `yield` invariant),
+// or the nudge/guidance would point at a tool the model cannot call. No memory
+// backend is configured (manage_skill needs only `autolearn.enabled`), so the
+// session starts without a heavy backend.
+describe("createAgentSession auto-learn tool activation", () => {
+ let registryDir: string;
+ let authStorage: AuthStorage;
+ let modelRegistry: ModelRegistry;
+ const sessions: AgentSession[] = [];
+
+ beforeAll(async () => {
+ registryDir = path.join(os.tmpdir(), `pi-autolearn-active-${Snowflake.next()}`);
+ fs.mkdirSync(registryDir, { recursive: true });
+ authStorage = await AuthStorage.create(path.join(registryDir, "auth.db"));
+ modelRegistry = new ModelRegistry(authStorage);
+ });
+
+ afterAll(async () => {
+ for (const session of sessions) await session.dispose().catch(() => {});
+ authStorage.close();
+ if (fs.existsSync(registryDir)) fs.rmSync(registryDir, { recursive: true, force: true });
+ });
+
+ async function activeToolNames(settings: Settings): Promise {
+ const { session } = await createAgentSession({
+ cwd: registryDir,
+ agentDir: registryDir,
+ modelRegistry,
+ sessionManager: SessionManager.inMemory(),
+ settings,
+ model: getBundledModel("openai", "gpt-4o-mini"),
+ disableExtensionDiscovery: true,
+ toolNames: ["read"],
+ });
+ sessions.push(session);
+ return session.getActiveToolNames();
+ }
+
+ it("activates force-included manage_skill in a restricted top-level session", async () => {
+ const names = await activeToolNames(Settings.isolated({ "autolearn.enabled": true }));
+ expect(names).toContain("read");
+ // Built by createTools' force-include AND activated by the SDK's explicit-list
+ // re-inclusion, so guidance/controller point at a callable tool.
+ expect(names).toContain("manage_skill");
+ });
+
+ it("omits manage_skill from a restricted session when auto-learn is off", async () => {
+ const names = await activeToolNames(Settings.isolated({}));
+ expect(names).toContain("read");
+ expect(names).not.toContain("manage_skill");
+ });
+});
From 1df62df8265bcf82b5206ea5d3fc238f9a8ce839 Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 14:00:20 +0900
Subject: [PATCH 14/14] test(coding-agent): cover learned-lesson injection
budget exhaustion
Grill test-gap finding: the learned-lesson read-back had no coverage for the
budget clamp when a memory summary already fills `summaryInjectionTokenLimit`.
Add a case with an oversized summary asserting the lesson is dropped and the
combined block stays bounded.
---
.../test/autolearn-learn-local.test.ts | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/packages/coding-agent/test/autolearn-learn-local.test.ts b/packages/coding-agent/test/autolearn-learn-local.test.ts
index d917bb696..7501e970e 100644
--- a/packages/coding-agent/test/autolearn-learn-local.test.ts
+++ b/packages/coding-agent/test/autolearn-learn-local.test.ts
@@ -197,6 +197,22 @@ describe("learned-lesson read-back", () => {
expect(out).not.toContain(token);
expect(out).toContain("[REDACTED]");
});
+
+ it("drops learned lessons when the summary already fills the injection budget", async () => {
+ const settings = Settings.isolated({ "memory.backend": "local" });
+ const root = getMemoryRoot(agentDir, settings.getCwd());
+ // A summary far larger than any injection budget: after truncation it
+ // consumes the whole token budget, leaving no room for lessons.
+ const hugeSummary = `${"summary ".repeat(50_000)}\n`;
+ await Bun.write(path.join(root, "memory_summary.md"), hugeSummary);
+ await saveLearnedLesson(agentDir, settings.getCwd(), { content: "UNIQUE_LESSON_MARKER" });
+ const out = await buildMemoryToolDeveloperInstructions(agentDir, settings);
+ expect(out).toBeDefined();
+ expect(out).toContain("summary"); // summary is still injected (truncated)
+ expect(out).not.toContain("UNIQUE_LESSON_MARKER"); // lesson dropped: budget exhausted
+ // The combined block stays bounded — it never grows to the raw summary size.
+ expect((out ?? "").length).toBeLessThan(hugeSummary.length);
+ });
});
describe("learn tool (local backend)", () => {