Say what a skill proposal changes, so a kept runbook never reads as saved

propose_skill took no strict schema, so a runbook sent under another key
was dropped and the saved one proposed again; the confirm still said
"Saved". Now an unknown key is an error, a change that changes nothing is
refused, and both the bot and the confirm outcome are told which fields
change and whether the runbook is among them.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

AGENTS.md

@@ -184,7 +184,7 @@ Read-only browser of every volume (`src/utils/storage-volumes.ts`: bots, workspa
 - **Calling:** `resolveHarnessCandidates` -> `withCandidates` with process-wide cooldowns; errors are `HarnessError`. Entry points under `text/`, `media/`, `audio/`, `image/`, `embeddings/`. `run-text-loop.ts` sticks to one model, caps steps (`grabHarnessMaxToolSteps`, `grabReplyMaxToolSteps`), trims old tool output to fit context. Models that can't take an image/audio part get it read by vision/transcription first.
 - **Per-bot models:** workspace row > bot row > default; the coordinator always uses the defaults. Every entry point takes `bot_id` and passes it on. Shared form `settings/models/(partials)/harness-settings-form.tsx`.
 - **Tools:** `HarnessTool` built per run; own tools validate with zod. `run-tool-call.ts` reports errors back to the model and caps each call (`AppData.HarnessToolTimeoutMs`). Bot tools in `src/harness/tools/bot/` (memory, instructions, tasks, schedules, `start_task`, coordinator's crew management, `delegate_task` and `hand_over_schedule`, with results relayed back by `relayJobToRequester`).
-- **Extensions:** MCP servers (`mcp_servers`, remote or stdio), custom tools (`custom_tools`: http, script, prompt) and skills (`skills`, runbooks loaded with `load_skill`). Given per bot / per workspace (`botExtensionsSql`); `all_bots` only decides what new bots get. Templates `{{args.x}}`/`{{env.KEY}}` via `fillTemplate`; secrets redacted from results. stdio servers run in a per-process pool. Skill body limits are app settings (`grabSkillBodyMaxChars`, `grabSkillBodyShownChars`).
+- **Extensions:** MCP servers (`mcp_servers`, remote or stdio), custom tools (`custom_tools`: http, script, prompt) and skills (`skills`, runbooks loaded with `load_skill`). Given per bot / per workspace (`botExtensionsSql`); `all_bots` only decides what new bots get. Templates `{{args.x}}`/`{{env.KEY}}` via `fillTemplate`; secrets redacted from results. stdio servers run in a per-process pool. Skill body limits are app settings (`grabSkillBodyMaxChars`, `grabSkillBodyShownChars`). A skill proposal says what it changes (`skillProposalChanges`) to the bot and in the confirm outcome, takes no unknown keys (a misnamed `body` would re-propose the saved runbook) and is refused when it changes nothing.
 - **Bots never add or repoint extensions or set envs.** The coordinator may only change what can't alter where data is sent (name, description, on/off, access). New skills, servers, tools, or changes to a URL, headers, kind, config or a skill's runbook go through `propose_*` -> `extension_proposals` -> `[proposal:ID]` card, which the app describes from the config itself (never from what the bot says) and the super admin confirms (`checkProposalData` re-checks). Stdio servers are never proposed by bots (they run in the app container, which holds the docker socket).
 - **Check before and after acting** (`ActionCheckNote`): look for an existing record before creating one, verify every action took effect. Tools enforce it (read-back on memory saves, duplicate checks on bots, schedules, tasks, workspaces).
 

src/functions/backend/db/extension-proposals/confirm-extension-proposal.ts

@@ -10,6 +10,7 @@ import grabSkill from "../extensions/grab-skill";
 import saveSkill from "../extensions/save-skill";
 import syncMcpServer from "../extensions/sync-mcp-server";
 import extensionEnvUses from "@/src/utils/extension-env-uses";
+import skillProposalChanges from "@/src/utils/skill-proposal-changes";
 import checkProposalData from "./check-proposal-data";
 import claimExtensionProposal from "./claim-extension-proposal";
 import grabExtensionProposal from "./grab-extension-proposal";
@@ -35,15 +36,16 @@ export default async function confirmExtensionProposal({ user_id, proposal_id }:
 
     try {
         if (proposal.kind == "skill") {
-            if (proposal.extension_id) await grabSkill({ skill_id: proposal.extension_id });
+            const current = proposal.extension_id ? await grabSkill({ skill_id: proposal.extension_id }) : null;
+            const changes = skillProposalChanges({ data: proposal.data, current });
 
             const result_id = await saveSkill({
                 user_id,
                 data: { ...proposal.data, skill_id: proposal.extension_id || undefined },
             });
             const outcome = `Saved as skill #${result_id} (slug ${proposal.data.slug}).${
-                !proposal.extension_id && !proposal.data.all_bots ? " No bot gets it yet." : ""
+                current ? describeSavedChanges({ fields: changes.fields }) : ""
-            }`;
+            }${!proposal.extension_id && !proposal.data.all_bots ? " No bot gets it yet." : ""}`;
 
             await BunSQLite.sql({
                 sql: `UPDATE extension_proposals SET outcome = ?, result_id = ?, updated_at = ? WHERE id = ?`,
@@ -120,3 +122,16 @@ export default async function confirmExtensionProposal({ user_id, proposal_id }:
 
     return proposal;
 }
+
+/**
+ * What a confirmed change to a skill changed, so a runbook that stayed the
+ * same never reads as saved.
+ */
+function describeSavedChanges({ fields }: { fields: string[] }) {
+    if (!fields.length) return " Nothing changed: it was already saved like this.";
+
+    const named = fields.length == 1 ? fields[0] : `${fields.slice(0, -1).join(", ")} and ${fields[fields.length - 1]}`;
+    const runbook = fields.includes("runbook") ? "" : " The runbook is unchanged.";
+
+    return ` Changed: ${named}.${runbook}`;
+}

src/harness/tools/bot/extensions/describe-proposal-for-bot.ts

@@ -1,8 +1,14 @@
 import type { ExtensionProposalView } from "@/src/types";
 import proposalRef from "@/src/utils/proposal-ref";
+import type { SkillProposalChanges } from "@/src/utils/skill-proposal-changes";
 
 type Params = {
     view: ExtensionProposalView;
+    /**
+     * For a change to a skill: what it makes different, so the bot knows
+     * whether its new runbook is in the proposal at all.
+     */
+    skill_changes?: SkillProposalChanges;
 };
 
 /**
@@ -10,7 +16,7 @@ type Params = {
  * its reply, and the facts the card shows (worked out by the app), so it
  * tells the user the same thing the card does.
  */
-export default function describeProposalForBot({ view }: Params) {
+export default function describeProposalForBot({ view, skill_changes }: Params) {
     const ref = proposalRef({ id: view.id });
     const keys = view.envs.length
         ? view.envs
@@ -25,6 +31,7 @@ export default function describeProposalForBot({ view }: Params) {
         return [
             `Proposed as ${ref}. Nothing is saved yet: the user confirms or rejects it from the card ${ref} becomes in your reply, so put ${ref} in your reply.`,
             `The card shows the whole runbook${view.extension_id ? " and what changed" : ""}, and what it points bots at: hosts: ${line("Hosts")}; tool names: ${line("Tool names")}; about secrets: ${line("About secrets")}.`,
+            ...(skill_changes ? [describeSkillChanges({ changes: skill_changes })] : []),
             `In your reply, say in plain words what the skill makes bots do, anything in it that sends something out or touches secrets, and where its content came from.`,
             `Don't say it's added: the user's next message says whether they confirmed it and what happened.`,
         ].join("\n");
@@ -37,3 +44,11 @@ export default function describeProposalForBot({ view }: Params) {
         `Don't say it's added: the user's next message says whether they confirmed it and what happened.`,
     ].join("\n");
 }
+
+function describeSkillChanges({ changes }: { changes: SkillProposalChanges }) {
+    const runbook = changes.body_changed
+        ? ` The runbook is the one you sent (${changes.body_chars} characters, was ${changes.was_body_chars}).`
+        : ` The runbook stays as saved: this proposal doesn't change it. If you meant to change it, propose again with the whole new runbook in body, and tell the user this card doesn't.`;
+
+    return `It changes: ${changes.fields.join(", ")}.${runbook}`;
+}

src/harness/tools/bot/extensions/propose-skill-tool.ts

@@ -7,6 +7,7 @@ import grabSkill from "@/src/functions/backend/db/extensions/grab-skill";
 import { SaveSkillRequestSchema } from "@/src/schemas/extension-schema";
 import parseWithSchema from "@/src/utils/parse-with-schema";
 import proposalRef from "@/src/utils/proposal-ref";
+import skillProposalChanges from "@/src/utils/skill-proposal-changes";
 import type { HarnessTool } from "../../../types";
 import assertSuperAdminRun from "./assert-super-admin-run";
 import describeProposalForBot from "./describe-proposal-for-bot";
@@ -46,7 +47,7 @@ type Args = {
 export default function proposeSkillTool({ user_id, bot_id, session_id, workspace_id, onPropose }: Params): HarnessTool<Args> {
     return {
         name: "propose_skill",
-        description: `Propose adding a skill (a runbook bots load with load_skill when its one-line description says it applies), or changing one's name, slug, description or body (skill_id), for the user to confirm. Nothing is saved until they press Confirm on the card. Changing one keeps every field you leave out; body replaces the whole runbook, so read it first with list_extensions and keep what still applies. A new one needs name, slug (lowercase words with hyphens, how bots load it), description and body. First review the runbook yourself and give it in research: what it makes bots do, any step that sends data, files or keys anywhere or reads secrets, and where the content came from (add the pages to sources if any of it came from one). Never put a key's value in a runbook. Check list_extensions first: if a skill already covers the job, propose a change to it instead.`,
+        description: `Propose adding a skill (a runbook bots load with load_skill when its one-line description says it applies), or changing one's name, slug, description or body (skill_id), for the user to confirm. Nothing is saved until they press Confirm on the card. Changing one keeps every field you leave out; body replaces the whole runbook, so read it first with list_extensions and keep what still applies: send the whole new runbook in body, and leave body out only to keep the saved one. A new one needs name, slug (lowercase words with hyphens, how bots load it), description and body. First review the runbook yourself and give it in research: what it makes bots do, any step that sends data, files or keys anywhere or reads secrets, and where the content came from (add the pages to sources if any of it came from one). Never put a key's value in a runbook. Check list_extensions first: if a skill already covers the job, propose a change to it instead.`,
         schema: z.object({
             skill_id: z.number().int().positive().optional(),
             name: z.string().trim().max(80).optional(),
@@ -56,7 +57,12 @@ export default function proposeSkillTool({ user_id, bot_id, session_id, workspac
             enabled: z.boolean().optional(),
             all_bots: z.boolean().optional(),
             ...SkillReviewFields,
-        }),
+        })
+            /**
+             * A runbook sent under another name (`runbook`, `content`) would
+             * otherwise be dropped, and the saved one proposed again.
+             */
+            .strict(),
         run: async ({ args }) => {
             await assertSuperAdminRun({ user_id });
 
@@ -74,7 +80,15 @@ export default function proposeSkillTool({ user_id, bot_id, session_id, workspac
                 },
             });
 
-            if (data.body != current?.body) await assertSkillBodyFits({ body: data.body });
+            const changes = skillProposalChanges({ data, current });
+
+            if (current && !changes.fields.length) {
+                throw new Error(
+                    `That changes nothing: it's skill #${current.id} exactly as saved${args.body === undefined ? " (you sent no body, so the runbook stayed the saved one)" : ""}. Send what should change, the whole new runbook in body.`,
+                );
+            }
+
+            if (changes.body_changed) await assertSkillBodyFits({ body: data.body });
 
             const id = await createExtensionProposal({
                 user_id,
@@ -89,7 +103,10 @@ export default function proposeSkillTool({ user_id, bot_id, session_id, workspac
 
             onPropose?.({ ref: proposalRef({ id }) });
 
-            return describeProposalForBot({ view: await grabExtensionProposalView({ proposal_id: id, user_id }) });
+            return describeProposalForBot({
+                view: await grabExtensionProposalView({ proposal_id: id, user_id }),
+                skill_changes: current ? changes : undefined,
+            });
         },
     };
 }

src/utils/skill-proposal-changes.test.ts

@@ -0,0 +1,51 @@
+import { describe, expect, test } from "bun:test";
+import type { SkillRecord } from "@/src/types";
+import skillProposalChanges from "./skill-proposal-changes";
+
+const current: SkillRecord = {
+    id: 2,
+    name: "WhatsApp Web API",
+    slug: "whatsapp-web-api",
+    description: "Old description",
+    body: "1. Install Baileys.\n2. Pair.",
+    all_bots: false,
+    enabled: true,
+};
+
+describe("skillProposalChanges", () => {
+    test("a change that kept the old runbook doesn't name the runbook", () => {
+        const changes = skillProposalChanges({ current, data: { ...current, skill_id: 2, description: "New description" } });
+
+        expect(changes.fields).toEqual(["description"]);
+        expect(changes.body_changed).toBe(false);
+    });
+
+    test("a new runbook is named, with both lengths", () => {
+        const body = "1. Install Baileys, @hapi/boom and pino.\n2. Pair.\n3. Allowlist.";
+        const changes = skillProposalChanges({ current, data: { ...current, skill_id: 2, body } });
+
+        expect(changes.fields).toEqual(["runbook"]);
+        expect(changes.body_chars).toBe(body.length);
+        expect(changes.was_body_chars).toBe(current.body.length);
+    });
+
+    test("saving it as it is changes nothing", () => {
+        expect(skillProposalChanges({ current, data: { ...current, skill_id: 2 } }).fields).toEqual([]);
+    });
+
+    test("turning it off and giving it to every bot are changes", () => {
+        const changes = skillProposalChanges({ current, data: { ...current, enabled: false, all_bots: true } });
+
+        expect(changes.fields).toEqual(["on/off", "every bot gets it"]);
+    });
+
+    test("a new skill is all new", () => {
+        const changes = skillProposalChanges({
+            current: null,
+            data: { name: "Tone", slug: "tone", description: "Every reply", body: "Be brief.", enabled: true, all_bots: false },
+        });
+
+        expect(changes.fields).toEqual(["name", "slug", "description", "runbook"]);
+        expect(changes.body_changed).toBe(true);
+    });
+});

src/utils/skill-proposal-changes.ts

@@ -0,0 +1,45 @@
+import type { SaveSkillRequest } from "@/src/schemas/extension-schema";
+import type { SkillRecord } from "@/src/types";
+
+type Params = {
+    data: SaveSkillRequest;
+    /**
+     * The skill as saved now. Null for a new one, where everything is new.
+     */
+    current: SkillRecord | null;
+};
+
+export type SkillProposalChanges = {
+    /**
+     * What a change would make different, in words ("name", "runbook", ...).
+     * Empty when confirming it would save the skill exactly as it is.
+     */
+    fields: string[];
+    body_changed: boolean;
+    body_chars: number;
+    was_body_chars: number;
+};
+
+/**
+ * What saving a proposed skill would change against the saved one, so the
+ * bot and the confirm outcome can say it plainly. A proposal whose runbook
+ * came out the same (the bot left out `body`) reads "runbook" nowhere here.
+ */
+export default function skillProposalChanges({ data, current }: Params): SkillProposalChanges {
+    const body_changed = !current || current.body != data.body;
+    const checks = [
+        { field: "name", changed: !current || current.name != data.name },
+        { field: "slug", changed: !current || current.slug != data.slug },
+        { field: "description", changed: !current || current.description != data.description },
+        { field: "runbook", changed: body_changed },
+        { field: "on/off", changed: !!current && !!current.enabled != data.enabled },
+        { field: "every bot gets it", changed: !!current && data.all_bots && !current.all_bots },
+    ];
+
+    return {
+        fields: checks.filter((check) => check.changed).map((check) => check.field),
+        body_changed,
+        body_chars: data.body.length,
+        was_body_chars: current?.body.length || 0,
+    };
+}