fix(skill): patch up skill diffing code (#758)

Co-authored-by: Letta <noreply@letta.com>
This commit is contained in:
Charles Packer
2026-01-30 18:08:56 -08:00
committed by GitHub
parent 33681a62b1
commit bb6ce1f2c8
8 changed files with 135 additions and 139 deletions

View File

@@ -4,6 +4,7 @@
*/
import type { CreateBlock } from "@letta-ai/letta-client/resources/blocks/blocks";
import { READ_ONLY_BLOCK_LABELS } from "./memoryConstants";
import { MEMORY_PROMPTS } from "./promptAssets";
/**
@@ -33,11 +34,7 @@ export type MemoryBlockLabel = (typeof MEMORY_BLOCK_LABELS)[number];
* Block labels that should be read-only (agent cannot modify via memory tools).
* These blocks are managed by specific tools (e.g., Skill tool for skills/loaded_skills).
*/
export const READ_ONLY_BLOCK_LABELS = [
"skills",
"loaded_skills",
"memory_filesystem",
] as const;
export { READ_ONLY_BLOCK_LABELS };
/**
* Block labels that should be isolated per-conversation.

View File

@@ -0,0 +1,5 @@
export const READ_ONLY_BLOCK_LABELS = [
"skills",
"loaded_skills",
"memory_filesystem",
] as const;

View File

@@ -673,6 +673,10 @@ export async function syncMemoryFilesystem(
const fileInSystem = !!systemFile;
const blockEntry = attachedBlock || detachedBlock;
const isAttached = !!attachedBlock;
const isReadOnlyLabel = (
READ_ONLY_BLOCK_LABELS as readonly string[]
).includes(label);
const effectiveReadOnly = !!blockEntry?.read_only || isReadOnlyLabel;
// Get directory for file operations
const fileDir = fileInSystem ? systemDir : detachedDir;
@@ -742,7 +746,7 @@ export async function syncMemoryFilesystem(
// Case 2: Block exists, no file
if (!fileEntry && blockEntry) {
// Read-only blocks: never delete/un-tag. Always recreate file instead.
if (blockEntry.read_only) {
if (effectiveReadOnly) {
const targetDir = isAttached ? systemDir : detachedDir;
const fileContent = renderBlockToFileContent(blockEntry);
await writeMemoryFile(targetDir, label, fileContent);
@@ -818,7 +822,7 @@ export async function syncMemoryFilesystem(
// Frontmatter-only change: update metadata even when body matches
if (fileChanged) {
// Read-only blocks: ignore local changes, overwrite file with API content
if (blockEntry.read_only) {
if (effectiveReadOnly) {
const fileContent = renderBlockToFileContent(blockEntry);
await writeMemoryFile(fileDir, label, fileContent);
updatedFiles.push(label);
@@ -910,7 +914,7 @@ export async function syncMemoryFilesystem(
// Also sync attachment status to match file location
if (fileChanged) {
// Read-only blocks: ignore local changes, overwrite file with API content
if (blockEntry.read_only) {
if (effectiveReadOnly) {
const fileContent = renderBlockToFileContent(blockEntry);
await writeMemoryFile(fileDir, label, fileContent);
updatedFiles.push(label);
@@ -1215,7 +1219,8 @@ export async function checkMemoryFilesystemStatus(
const fileContent = systemFile?.content ?? detachedFile?.content ?? null;
const blockValue = attachedBlock?.value ?? detachedBlock?.value ?? null;
const blockReadOnly =
attachedBlock?.read_only ?? detachedBlock?.read_only ?? false;
(attachedBlock?.read_only ?? detachedBlock?.read_only ?? false) ||
(READ_ONLY_BLOCK_LABELS as readonly string[]).includes(label);
const fileInSystem = !!systemFile;
const isAttached = !!attachedBlock;
@@ -1301,6 +1306,11 @@ function classifyLabel(
}
if (fileContent === null && blockValue !== null) {
if (blockReadOnly) {
// Read-only blocks: missing file should be recreated
pendingFromFile.push(label);
return;
}
if (lastFileHash && !blockChanged) {
// File was deleted, block unchanged — would delete block
return;

View File

@@ -0,0 +1,47 @@
import { createHash } from "node:crypto";
import { READ_ONLY_BLOCK_LABELS } from "../../../../../agent/memoryConstants";
/**
* Read-only block labels. These are API-authoritative.
*/
export const READ_ONLY_LABELS = new Set(
READ_ONLY_BLOCK_LABELS as readonly string[],
);
/**
* Parse MDX-style frontmatter from content.
* This is a copy of parseMdxFrontmatter from src/agent/memory.ts.
* The test ensures this stays in sync with the original.
*/
export function parseFrontmatter(content: string): {
frontmatter: Record<string, string>;
body: string;
} {
const frontmatterRegex = /^---\n([\s\S]*?)\n---\n([\s\S]*)$/;
const match = content.match(frontmatterRegex);
if (!match || !match[1] || !match[2]) {
return { frontmatter: {}, body: content };
}
const frontmatterText = match[1];
const body = match[2];
const frontmatter: Record<string, string> = {};
// Parse YAML-like frontmatter (simple key: value pairs)
for (const line of frontmatterText.split("\n")) {
const colonIndex = line.indexOf(":");
if (colonIndex > 0) {
const key = line.slice(0, colonIndex).trim();
const value = line.slice(colonIndex + 1).trim();
frontmatter[key] = value;
}
}
return { frontmatter, body: body.trim() };
}
export function hashFileBody(content: string): string {
const { body } = parseFrontmatter(content);
return createHash("sha256").update(body).digest("hex");
}

View File

@@ -18,6 +18,7 @@ import { readdir, readFile } from "node:fs/promises";
import { createRequire } from "node:module";
import { homedir } from "node:os";
import { join, normalize, relative } from "node:path";
import { hashFileBody, READ_ONLY_LABELS } from "./lib/frontmatter";
const require = createRequire(import.meta.url);
const Letta = require("@letta-ai/letta-client")
@@ -57,49 +58,7 @@ function hashContent(content: string): string {
return createHash("sha256").update(content).digest("hex");
}
/**
* Parse frontmatter from file content.
*/
function parseFrontmatter(content: string): {
frontmatter: Record<string, string>;
body: string;
} {
const frontmatterRegex = /^---\n([\s\S]*?)\n---\n([\s\S]*)$/;
const match = content.match(frontmatterRegex);
if (!match || !match[1] || !match[2]) {
return { frontmatter: {}, body: content };
}
const frontmatterText = match[1];
const body = match[2];
const frontmatter: Record<string, string> = {};
for (const line of frontmatterText.split("\n")) {
const colonIdx = line.indexOf(":");
if (colonIdx > 0) {
const key = line.slice(0, colonIdx).trim();
let value = line.slice(colonIdx + 1).trim();
if (
(value.startsWith('"') && value.endsWith('"')) ||
(value.startsWith("'") && value.endsWith("'"))
) {
value = value.slice(1, -1);
}
frontmatter[key] = value;
}
}
return { frontmatter, body };
}
/**
* Hash just the body of file content (excluding frontmatter).
*/
function hashFileBody(content: string): string {
const { body } = parseFrontmatter(content);
return hashContent(body);
}
// parseFrontmatter/hashFileBody provided by shared helper
function getMemoryRoot(agentId: string): string {
return join(homedir(), ".letta", "agents", agentId, "memory");
@@ -308,7 +267,9 @@ async function findConflicts(agentId: string): Promise<{
if (!fileEntry || !blockEntry) continue;
// read_only blocks are API-authoritative; no conflicts possible
if (blockEntry.read_only) continue;
const effectiveReadOnly =
!!blockEntry.read_only || READ_ONLY_LABELS.has(label);
if (effectiveReadOnly) continue;
// Full file hash for "file changed" check
const fileHash = hashContent(fileEntry.content);

View File

@@ -23,6 +23,7 @@ import { readdir, readFile } from "node:fs/promises";
import { createRequire } from "node:module";
import { homedir } from "node:os";
import { dirname, join, relative } from "node:path";
import { parseFrontmatter, READ_ONLY_LABELS } from "./lib/frontmatter";
const require = createRequire(import.meta.url);
const Letta = require("@letta-ai/letta-client")
@@ -67,41 +68,7 @@ function hashContent(content: string): string {
return createHash("sha256").update(content).digest("hex");
}
/**
* Parse frontmatter from file content.
*/
function parseFrontmatter(content: string): {
frontmatter: Record<string, string>;
body: string;
} {
const frontmatterRegex = /^---\n([\s\S]*?)\n---\n([\s\S]*)$/;
const match = content.match(frontmatterRegex);
if (!match || !match[1] || !match[2]) {
return { frontmatter: {}, body: content };
}
const frontmatterText = match[1];
const body = match[2];
const frontmatter: Record<string, string> = {};
for (const line of frontmatterText.split("\n")) {
const colonIdx = line.indexOf(":");
if (colonIdx > 0) {
const key = line.slice(0, colonIdx).trim();
let value = line.slice(colonIdx + 1).trim();
if (
(value.startsWith('"') && value.endsWith('"')) ||
(value.startsWith("'") && value.endsWith("'"))
) {
value = value.slice(1, -1);
}
frontmatter[key] = value;
}
}
return { frontmatter, body };
}
// parseFrontmatter provided by shared helper
/**
* Parse block update from file content (update-mode: only update metadata if present in frontmatter).
@@ -355,9 +322,12 @@ async function resolveConflicts(
continue;
}
const effectiveReadOnly =
!!block.read_only || READ_ONLY_LABELS.has(label);
if (resolution === "file") {
// read_only blocks: ignore local edits, overwrite file from API
if (block.read_only) {
if (effectiveReadOnly) {
const fileContent = renderBlockToFileContent(block);
writeMemoryFile(dir, label, fileContent);
result.resolved.push({

View File

@@ -18,6 +18,7 @@ import { readdir, readFile } from "node:fs/promises";
import { createRequire } from "node:module";
import { homedir } from "node:os";
import { join, relative } from "node:path";
import { hashFileBody, READ_ONLY_LABELS } from "./lib/frontmatter";
const require = createRequire(import.meta.url);
const Letta = require("@letta-ai/letta-client")
@@ -57,49 +58,7 @@ function hashContent(content: string): string {
return createHash("sha256").update(content).digest("hex");
}
/**
* Parse frontmatter from file content.
*/
function parseFrontmatter(content: string): {
frontmatter: Record<string, string>;
body: string;
} {
const frontmatterRegex = /^---\n([\s\S]*?)\n---\n([\s\S]*)$/;
const match = content.match(frontmatterRegex);
if (!match || !match[1] || !match[2]) {
return { frontmatter: {}, body: content };
}
const frontmatterText = match[1];
const body = match[2];
const frontmatter: Record<string, string> = {};
for (const line of frontmatterText.split("\n")) {
const colonIdx = line.indexOf(":");
if (colonIdx > 0) {
const key = line.slice(0, colonIdx).trim();
let value = line.slice(colonIdx + 1).trim();
if (
(value.startsWith('"') && value.endsWith('"')) ||
(value.startsWith("'") && value.endsWith("'"))
) {
value = value.slice(1, -1);
}
frontmatter[key] = value;
}
}
return { frontmatter, body };
}
/**
* Hash just the body of file content (excluding frontmatter).
*/
function hashFileBody(content: string): string {
const { body } = parseFrontmatter(content);
return hashContent(body);
}
// parseFrontmatter/hashFileBody provided by shared helper
function getMemoryRoot(agentId: string): string {
return join(homedir(), ".letta", "agents", agentId, "memory");
@@ -175,12 +134,6 @@ async function readMemoryFiles(
// Only memory_filesystem is managed by memfs itself
const MEMFS_MANAGED_LABELS = new Set(["memory_filesystem"]);
// Read-only labels are API-authoritative (file-only copies should be ignored)
const READ_ONLY_LABELS = new Set([
"skills",
"loaded_skills",
"memory_filesystem",
]);
interface StatusResult {
conflicts: Array<{ label: string }>;
@@ -299,6 +252,8 @@ async function checkStatus(agentId: string): Promise<StatusResult> {
const fileInSystem = !!systemFile;
const blockEntry = attachedBlock || detachedBlock;
const isAttached = !!attachedBlock;
const effectiveReadOnly =
!!blockEntry?.read_only || READ_ONLY_LABELS.has(label);
// Check for location mismatch
if (fileEntry && blockEntry) {
@@ -331,6 +286,10 @@ async function checkStatus(agentId: string): Promise<StatusResult> {
}
if (!fileEntry && blockEntry) {
if (effectiveReadOnly) {
pendingFromFile.push(label);
continue;
}
if (lastFileHash && !blockChanged) continue; // File deleted, block unchanged
newBlocks.push(label);
continue;
@@ -339,7 +298,7 @@ async function checkStatus(agentId: string): Promise<StatusResult> {
if (!fileEntry || !blockEntry) continue;
// Both exist - read_only blocks are API-authoritative
if (blockEntry.read_only) {
if (effectiveReadOnly) {
if (blockChanged) pendingFromBlock.push(label);
continue;
}

View File

@@ -0,0 +1,47 @@
/**
* Tests for syncing-memory-filesystem script helpers
*/
import { describe, expect, test } from "bun:test";
import { createHash } from "node:crypto";
import { parseMdxFrontmatter } from "../../agent/memory";
import {
hashFileBody,
parseFrontmatter,
} from "../../skills/builtin/syncing-memory-filesystem/scripts/lib/frontmatter";
describe("syncing-memory-filesystem script frontmatter helpers", () => {
test("parseFrontmatter matches parseMdxFrontmatter and trims body", () => {
const content = [
"---",
"description: Test description",
"limit: 123",
"---",
"",
"Hello world",
"",
].join("\n");
const parsed = parseFrontmatter(content);
const expected = parseMdxFrontmatter(content);
expect(parsed).toEqual(expected);
expect(parsed.body).toBe("Hello world");
});
test("hashFileBody uses trimmed body (no leading newline)", () => {
const content = [
"---",
"description: Test description",
"---",
"",
"Line one",
"Line two",
].join("\n");
const { body } = parseMdxFrontmatter(content);
const expectedHash = createHash("sha256").update(body).digest("hex");
expect(hashFileBody(content)).toBe(expectedHash);
});
});