diff --git a/src/lib/agent-yaml.test.ts b/src/lib/agent-yaml.test.ts new file mode 100644 index 000000000..06fdbb7e3 --- /dev/null +++ b/src/lib/agent-yaml.test.ts @@ -0,0 +1,362 @@ +/** + * Agent YAML — Unit Tests + * + * Covers: + * - Round-trip: writeAgentYaml → parseAgentYaml returns the same config + * after stripping the derived fields (name, dir, registeredAt). + * - Strict schema: unknown top-level keys fail with the key named. + * - Scope guard: `skill` and `extraArgs` are rejected (out of scope for + * this wish — they live in the `agent_templates` PG row, not the file). + * - Nested: `permissions.bashAllowPatterns` parses; `permissions.bashAllow` + * (wrong name) throws an unknown-field error. + * - extractFrontmatterFromAgentsMd purity + byte-for-byte body fidelity + * (CRLF, trailing newlines, Unicode, "no frontmatter" case). + * - Concurrent writeAgentYaml calls on the same path yield exactly one + * winner — never a partial/spliced file. + * - Malformed YAML surfaces a clear error. + * + * Run with: bun test src/lib/agent-yaml.test.ts + */ + +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + type AgentConfig, + AgentConfigSchema, + extractFrontmatterFromAgentsMd, + parseAgentYaml, + writeAgentYaml, +} from './agent-yaml.js'; + +// --------------------------------------------------------------------------- +// Fixture helpers +// --------------------------------------------------------------------------- + +let tempDir: string; + +beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), 'agent-yaml-test-')); +}); + +afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); +}); + +function tmpYaml(name = 'agent.yaml'): string { + return join(tempDir, name); +} + +// --------------------------------------------------------------------------- +// Round-trip +// --------------------------------------------------------------------------- + +describe('writeAgentYaml / parseAgentYaml round-trip', () => { + test('round-trips a fully-populated config (minus derived fields)', async () => { + const path = tmpYaml(); + const input: AgentConfig = { + repo: '/home/user/repo', + team: 'my-team', + promptMode: 'append', + model: 'sonnet', + roles: ['engineer', 'reviewer'], + omniAgentId: 'uuid-1234', + description: 'My agent', + color: 'blue', + provider: 'claude-sdk', + permissions: { + preset: 'strict', + allow: ['Read', 'Grep'], + deny: ['Bash'], + bashAllowPatterns: ['git status', 'ls *'], + }, + disallowedTools: ['WebSearch'], + omniScopes: ['say', 'react'], + hooks: { PreToolUse: [{ matcher: 'Read' }] }, + sdk: { + permissionMode: 'default', + maxTurns: 50, + allowedTools: ['Read', 'Write'], + settingSources: ['user', 'project'], + }, + }; + + await writeAgentYaml(path, input); + const parsed = await parseAgentYaml(path); + expect(parsed).toEqual(input); + }); + + test('strips derived fields (name, dir, registeredAt) from on-disk YAML', async () => { + const path = tmpYaml(); + const input: AgentConfig = { + name: 'derived-name', + dir: '/some/absolute/path', + registeredAt: '2026-01-01T00:00:00Z', + model: 'opus', + promptMode: 'append', + }; + + await writeAgentYaml(path, input); + const onDisk = await readFile(path, 'utf-8'); + + // The raw YAML file must NOT contain any of the derived keys. + expect(onDisk).not.toContain('name:'); + expect(onDisk).not.toContain('dir:'); + expect(onDisk).not.toContain('registeredAt:'); + + // Non-derived fields must still round-trip. + expect(onDisk).toContain('model: opus'); + expect(onDisk).toContain('promptMode: append'); + + const parsed = await parseAgentYaml(path); + expect(parsed.name).toBeUndefined(); + expect(parsed.dir).toBeUndefined(); + expect(parsed.registeredAt).toBeUndefined(); + expect(parsed.model).toBe('opus'); + expect(parsed.promptMode).toBe('append'); + }); + + test('round-trips an empty config', async () => { + const path = tmpYaml(); + await writeAgentYaml(path, {}); + const parsed = await parseAgentYaml(path); + expect(parsed).toEqual({}); + }); +}); + +// --------------------------------------------------------------------------- +// Strict schema / scope guard +// --------------------------------------------------------------------------- + +describe('AgentConfigSchema strict mode', () => { + test('rejects an unknown top-level key with the key named', () => { + const result = AgentConfigSchema.safeParse({ model: 'sonnet', bogus: 42 }); + expect(result.success).toBe(false); + if (result.success) return; + const message = result.error.issues + .map((i) => `${i.code}:${JSON.stringify((i as any).keys ?? i.message)}`) + .join('|'); + expect(message).toContain('bogus'); + }); + + test('rejects `skill` top-level key (scope guard for agent_templates DB field)', () => { + const result = AgentConfigSchema.safeParse({ skill: 'whatever' }); + expect(result.success).toBe(false); + if (result.success) return; + const flat = JSON.stringify(result.error.issues); + expect(flat).toContain('skill'); + }); + + test('rejects `extraArgs` top-level key (scope guard for agent_templates DB field)', () => { + const result = AgentConfigSchema.safeParse({ extraArgs: ['--foo'] }); + expect(result.success).toBe(false); + if (result.success) return; + const flat = JSON.stringify(result.error.issues); + expect(flat).toContain('extraArgs'); + }); + + test('parseAgentYaml surfaces the unknown-key error message when skill is present', async () => { + const path = tmpYaml('scope-guard.yaml'); + await writeFile(path, 'skill: something\nmodel: sonnet\n', 'utf-8'); + await expect(parseAgentYaml(path)).rejects.toThrow(/skill/); + }); +}); + +// --------------------------------------------------------------------------- +// Nested fields +// --------------------------------------------------------------------------- + +describe('permissions nested schema', () => { + test('accepts permissions.bashAllowPatterns (correct name)', () => { + const result = AgentConfigSchema.safeParse({ + permissions: { + bashAllowPatterns: ['git status', 'ls'], + }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.permissions?.bashAllowPatterns).toEqual(['git status', 'ls']); + } + }); + + test('rejects permissions.bashAllow (wrong field name)', () => { + const result = AgentConfigSchema.safeParse({ + permissions: { + bashAllow: ['git status'], + }, + }); + expect(result.success).toBe(false); + if (result.success) return; + const flat = JSON.stringify(result.error.issues); + expect(flat).toContain('bashAllow'); + // And the error should be pinned to the nested `permissions` path. + expect(flat).toContain('permissions'); + }); + + test('rejects an unknown nested key even when sibling fields are valid', () => { + const result = AgentConfigSchema.safeParse({ + permissions: { + allow: ['Read'], + unknown: true, + }, + }); + expect(result.success).toBe(false); + if (result.success) return; + const flat = JSON.stringify(result.error.issues); + expect(flat).toContain('unknown'); + }); +}); + +// --------------------------------------------------------------------------- +// extractFrontmatterFromAgentsMd — purity + byte-for-byte body fidelity +// --------------------------------------------------------------------------- + +describe('extractFrontmatterFromAgentsMd', () => { + test('returns full body when no frontmatter fence is present', () => { + const content = '# Hello\n\nThis is the body.\n'; + const { frontmatter, body } = extractFrontmatterFromAgentsMd(content); + expect(frontmatter).toBeNull(); + expect(body).toBe(content); + }); + + test('extracts basic frontmatter and returns body starting after the closing fence', () => { + const content = '---\nname: test\nmodel: sonnet\n---\n# Body\n'; + const { frontmatter, body } = extractFrontmatterFromAgentsMd(content); + expect(frontmatter).toBe('name: test\nmodel: sonnet'); + expect(body).toBe('# Body\n'); + }); + + test('preserves CRLF line endings in the body byte-for-byte', () => { + // Frontmatter itself uses LF (YAML convention); body preserves CRLF. + const content = '---\nname: test\n---\n# Body\r\n\r\nLine two\r\n'; + const { frontmatter, body } = extractFrontmatterFromAgentsMd(content); + expect(frontmatter).toBe('name: test'); + expect(body).toBe('# Body\r\n\r\nLine two\r\n'); + }); + + test('preserves trailing newlines exactly', () => { + const content = '---\nname: test\n---\nbody line\n\n\n'; + const { body } = extractFrontmatterFromAgentsMd(content); + expect(body).toBe('body line\n\n\n'); + }); + + test('preserves Unicode characters (emoji, Portuguese accents, CJK)', () => { + const body = 'Olá, Genie! 🧞 こんにちは — café\n'; + const content = `---\nname: unicode\n---\n${body}`; + const result = extractFrontmatterFromAgentsMd(content); + expect(result.frontmatter).toBe('name: unicode'); + expect(result.body).toBe(body); + }); + + test('returns no-frontmatter when content does not start with `---\\n`', () => { + // Leading space/newline/BOM must NOT match. + const leadingSpace = ' ---\nname: test\n---\nbody'; + expect(extractFrontmatterFromAgentsMd(leadingSpace).frontmatter).toBeNull(); + expect(extractFrontmatterFromAgentsMd(leadingSpace).body).toBe(leadingSpace); + + const leadingNewline = '\n---\nname: test\n---\nbody'; + expect(extractFrontmatterFromAgentsMd(leadingNewline).frontmatter).toBeNull(); + expect(extractFrontmatterFromAgentsMd(leadingNewline).body).toBe(leadingNewline); + }); + + test('returns no-frontmatter when the closing fence is missing', () => { + const content = '---\nname: test\nbody-but-no-fence'; + const { frontmatter, body } = extractFrontmatterFromAgentsMd(content); + expect(frontmatter).toBeNull(); + expect(body).toBe(content); + }); + + test('is pure — calling twice returns equal results without side effects', () => { + const content = '---\nname: pure\n---\nbody\n'; + const a = extractFrontmatterFromAgentsMd(content); + const b = extractFrontmatterFromAgentsMd(content); + expect(a).toEqual(b); + // The input string must not be mutated. + expect(content).toBe('---\nname: pure\n---\nbody\n'); + }); + + test('handles empty frontmatter block', () => { + const content = '---\n---\nbody\n'; + const { frontmatter, body } = extractFrontmatterFromAgentsMd(content); + expect(frontmatter).toBe(''); + expect(body).toBe('body\n'); + }); +}); + +// --------------------------------------------------------------------------- +// Concurrent writes +// --------------------------------------------------------------------------- + +describe('concurrent writeAgentYaml', () => { + test('two simultaneous writers produce exactly one winner — never a splice', async () => { + const path = tmpYaml('concurrent.yaml'); + + const a: AgentConfig = { + model: 'opus', + description: 'alpha '.repeat(200).trim(), + }; + const b: AgentConfig = { + model: 'sonnet', + description: 'bravo '.repeat(200).trim(), + }; + + // Kick off both writes in parallel and await both. One will acquire the + // lock first and persist; the other will block, acquire after, and overwrite. + // Either way, the final file must parse cleanly and equal exactly one input. + await Promise.all([writeAgentYaml(path, a), writeAgentYaml(path, b)]); + + const parsed = await parseAgentYaml(path); + // The parsed result must be EXACTLY one of the two inputs. + const matchA = parsed.model === a.model && parsed.description === a.description; + const matchB = parsed.model === b.model && parsed.description === b.description; + expect(matchA || matchB).toBe(true); + expect(matchA && matchB).toBe(false); // mutually exclusive + }); + + test('many parallel writers all complete and yield a parseable file', async () => { + const path = tmpYaml('many-concurrent.yaml'); + const configs: AgentConfig[] = Array.from({ length: 8 }, (_, i) => ({ + model: `model-${i}`, + description: `writer ${i}`, + })); + + await Promise.all(configs.map((cfg) => writeAgentYaml(path, cfg))); + + const parsed = await parseAgentYaml(path); + // Result must match EXACTLY one of the configs. + const hits = configs.filter((c) => c.model === parsed.model && c.description === parsed.description); + expect(hits).toHaveLength(1); + }); +}); + +// --------------------------------------------------------------------------- +// Malformed YAML +// --------------------------------------------------------------------------- + +describe('parseAgentYaml error handling', () => { + test('throws a clear error on malformed YAML', async () => { + const path = tmpYaml('malformed.yaml'); + // Unclosed quote — js-yaml will throw. + await writeFile(path, 'model: "unclosed\n', 'utf-8'); + await expect(parseAgentYaml(path)).rejects.toThrow(/Malformed YAML/); + }); + + test('throws when the file is missing', async () => { + const path = tmpYaml('does-not-exist.yaml'); + await expect(parseAgentYaml(path)).rejects.toThrow(/Failed to read/); + }); + + test('throws when the top-level YAML is an array rather than a mapping', async () => { + const path = tmpYaml('array.yaml'); + await writeFile(path, '- foo\n- bar\n', 'utf-8'); + await expect(parseAgentYaml(path)).rejects.toThrow(/must be a YAML mapping/); + }); + + test('treats an empty file as an empty config', async () => { + const path = tmpYaml('empty.yaml'); + await writeFile(path, '', 'utf-8'); + const parsed = await parseAgentYaml(path); + expect(parsed).toEqual({}); + }); +}); diff --git a/src/lib/agent-yaml.ts b/src/lib/agent-yaml.ts new file mode 100644 index 000000000..4db2324db --- /dev/null +++ b/src/lib/agent-yaml.ts @@ -0,0 +1,432 @@ +/** + * Agent YAML — Single source of truth for `agent.yaml` schema, parsing, and writing. + * + * Mirrors {@link DirectoryEntry} (src/lib/agent-directory.ts) field-for-field + * using the exact TypeScript field names. Zod schema is `.strict()` at every + * object level so unknown keys are rejected with a clear, field-named error. + * + * Derived fields: + * - `name` — derived from the agent directory name + * - `dir` — derived from the absolute path where the yaml lives + * - `registeredAt` — derived from file mtime + * + * All three are accepted by the schema for round-trip ergonomics but are + * STRIPPED by {@link writeAgentYaml} before serialization so they never leak + * into the on-disk representation. + * + * Concurrent writes are safe: `writeAgentYaml` uses the shared `lockfile` + * module to serialize writers and writes atomically via `${path}.tmp` + + * `rename`, so a reader can never observe a partial/spliced file. + * + * Scope guard: top-level keys that are OUT of scope for the current wish + * (notably `skill`, `extraArgs`) are rejected by `.strict()` with an error + * that names the offending key. + */ + +import { readFile, rename, writeFile } from 'node:fs/promises'; +import * as yaml from 'js-yaml'; +import { z } from 'zod'; +import { acquireLock, releaseLock } from './lockfile.js'; + +// ============================================================================ +// Zod Schemas — mirror SdkDirectoryConfig + DirectoryEntry +// ============================================================================ + +// Primitive / union building blocks from SdkDirectoryConfig. +const SdkPermissionModeSchema = z.enum([ + 'default', + 'acceptEdits', + 'bypassPermissions', + 'plan', + 'dontAsk', + 'auto', + 'remoteApproval', +]); + +const SdkEffortLevelSchema = z.union([z.enum(['low', 'medium', 'high', 'max']), z.number()]); + +const SdkThinkingConfigSchema = z.union([ + z.object({ type: z.literal('adaptive') }).strict(), + z.object({ type: z.literal('enabled'), budgetTokens: z.number().optional() }).strict(), + z.object({ type: z.literal('disabled') }).strict(), +]); + +const SdkMcpStdioServerConfigSchema = z + .object({ + type: z.literal('stdio').optional(), + command: z.string(), + args: z.array(z.string()).optional(), + env: z.record(z.string()).optional(), + }) + .strict(); + +const SdkMcpSSEServerConfigSchema = z + .object({ + type: z.literal('sse'), + url: z.string(), + headers: z.record(z.string()).optional(), + }) + .strict(); + +const SdkMcpHttpServerConfigSchema = z + .object({ + type: z.literal('http'), + url: z.string(), + headers: z.record(z.string()).optional(), + }) + .strict(); + +const SdkMcpServerConfigSchema = z.union([ + SdkMcpStdioServerConfigSchema, + SdkMcpSSEServerConfigSchema, + SdkMcpHttpServerConfigSchema, +]); + +const SdkSubagentConfigSchema = z + .object({ + description: z.string(), + prompt: z.string(), + tools: z.array(z.string()).optional(), + disallowedTools: z.array(z.string()).optional(), + model: z.string().optional(), + mcpServers: z.array(z.union([z.string(), z.record(SdkMcpStdioServerConfigSchema)])).optional(), + skills: z.array(z.string()).optional(), + maxTurns: z.number().optional(), + background: z.boolean().optional(), + memory: z.enum(['user', 'project', 'local']).optional(), + effort: SdkEffortLevelSchema.optional(), + permissionMode: SdkPermissionModeSchema.optional(), + }) + .strict(); + +const SdkCustomToolConfigSchema = z + .object({ + name: z.string(), + description: z.string(), + inputSchema: z.record(z.unknown()), + handler: z.string().optional(), + }) + .strict(); + +const SdkOutputFormatSchema = z + .object({ + type: z.literal('json_schema'), + schema: z.record(z.unknown()), + }) + .strict(); + +const SdkPluginConfigSchema = z + .object({ + type: z.literal('local'), + path: z.string(), + }) + .strict(); + +const SdkSandboxConfigSchema = z + .object({ + enabled: z.boolean().optional(), + autoAllowBashIfSandboxed: z.boolean().optional(), + failIfUnavailable: z.boolean().optional(), + network: z + .object({ + allowLocalBinding: z.boolean().optional(), + allowUnixSockets: z.array(z.string()).optional(), + }) + .strict() + .optional(), + }) + .strict(); + +const SdkHookMatcherConfigSchema = z + .object({ + toolName: z.string().optional(), + agentName: z.string().optional(), + }) + .strict(); + +const SdkHookEventSchema = z.enum([ + 'PreToolUse', + 'PostToolUse', + 'PostToolUseFailure', + 'Notification', + 'UserPromptSubmit', + 'SessionStart', + 'SessionEnd', + 'Stop', + 'StopFailure', + 'SubagentStart', + 'SubagentStop', + 'PreCompact', + 'PostCompact', + 'PermissionRequest', + 'PermissionDenied', + 'Setup', + 'TeammateIdle', + 'TaskCreated', + 'TaskCompleted', + 'Elicitation', + 'ElicitationResult', + 'ConfigChange', + 'WorktreeCreate', + 'WorktreeRemove', + 'InstructionsLoaded', + 'CwdChanged', + 'FileChanged', +]); + +const SdkBetaSchema = z.enum(['context-1m-2025-08-07']); + +const SdkSystemPromptSchema = z.union([ + z.string(), + z + .object({ + type: z.literal('preset'), + preset: z.literal('claude_code'), + append: z.string().optional(), + }) + .strict(), +]); + +const SdkDirectoryConfigSchema = z + .object({ + permissionMode: SdkPermissionModeSchema.optional(), + tools: z + .union([z.array(z.string()), z.object({ type: z.literal('preset'), preset: z.literal('claude_code') }).strict()]) + .optional(), + allowedTools: z.array(z.string()).optional(), + disallowedTools: z.array(z.string()).optional(), + maxTurns: z.number().optional(), + maxBudgetUsd: z.number().optional(), + effort: SdkEffortLevelSchema.optional(), + thinking: SdkThinkingConfigSchema.optional(), + agent: z.string().optional(), + agents: z.record(SdkSubagentConfigSchema).optional(), + mcpServers: z.record(SdkMcpServerConfigSchema).optional(), + plugins: z.array(SdkPluginConfigSchema).optional(), + customTools: z.array(SdkCustomToolConfigSchema).optional(), + persistSession: z.boolean().optional(), + enableFileCheckpointing: z.boolean().optional(), + outputFormat: SdkOutputFormatSchema.optional(), + includePartialMessages: z.boolean().optional(), + includeHookEvents: z.boolean().optional(), + promptSuggestions: z.boolean().optional(), + agentProgressSummaries: z.boolean().optional(), + systemPrompt: SdkSystemPromptSchema.optional(), + sandbox: SdkSandboxConfigSchema.optional(), + betas: z.array(SdkBetaSchema).optional(), + settingSources: z.array(z.enum(['user', 'project', 'local'])).optional(), + settings: z.union([z.string(), z.record(z.unknown())]).optional(), + hooks: z.record(SdkHookEventSchema, z.array(SdkHookMatcherConfigSchema)).optional(), + }) + .strict(); + +/** + * Zod schema mirroring `DirectoryEntry` from `src/lib/agent-directory.ts`. + * + * All object levels are `.strict()` so unknown keys (including out-of-scope + * fields like `skill` or `extraArgs`) produce an error naming the offender. + * + * The three derived fields (`name`, `dir`, `registeredAt`) are accepted on + * parse for round-trip ergonomics but {@link writeAgentYaml} strips them + * before serialization so they never land on disk. + */ +export const AgentConfigSchema = z + .object({ + name: z.string().optional(), + dir: z.string().optional(), + repo: z.string().optional(), + team: z.string().optional(), + promptMode: z.enum(['system', 'append']).optional(), + model: z.string().optional(), + roles: z.array(z.string()).optional(), + omniAgentId: z.string().optional(), + registeredAt: z.string().optional(), + description: z.string().optional(), + color: z.string().optional(), + provider: z.string().optional(), + permissions: z + .object({ + preset: z.string().optional(), + allow: z.array(z.string()).optional(), + deny: z.array(z.string()).optional(), + bashAllowPatterns: z.array(z.string()).optional(), + }) + .strict() + .optional(), + disallowedTools: z.array(z.string()).optional(), + omniScopes: z.array(z.string()).optional(), + hooks: z.record(z.unknown()).optional(), + sdk: SdkDirectoryConfigSchema.optional(), + }) + .strict(); + +/** + * The parsed, validated shape of an `agent.yaml` file. + * + * All fields are optional because derived fields (`name`, `dir`, `registeredAt`) + * are NOT written to disk and callers may want to construct partial configs. + * Callers requiring a fully-specified `DirectoryEntry` should hydrate the + * derived fields from filesystem context. + */ +export type AgentConfig = z.infer; + +// ============================================================================ +// Derived-field handling +// ============================================================================ + +/** Keys derived from filesystem context — stripped before writing to disk. */ +const DERIVED_KEYS = ['name', 'dir', 'registeredAt'] as const; + +function stripDerivedFields(config: AgentConfig): AgentConfig { + const out: Record = { ...config }; + for (const key of DERIVED_KEYS) { + delete out[key]; + } + return out as AgentConfig; +} + +// ============================================================================ +// Parsing +// ============================================================================ + +/** + * Read, parse, and validate an `agent.yaml` file at the given absolute path. + * + * Throws a descriptive `Error` on: + * - I/O failures (file missing, unreadable) + * - Malformed YAML (surfaces the js-yaml error verbatim) + * - Schema violations (surfaces the Zod error, which names the offending + * path including any unknown key or wrong type) + */ +export async function parseAgentYaml(path: string): Promise { + let raw: string; + try { + raw = await readFile(path, 'utf-8'); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + throw new Error(`Failed to read agent.yaml at ${path}: ${message}`); + } + + let parsed: unknown; + try { + parsed = yaml.load(raw); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + throw new Error(`Malformed YAML in ${path}: ${message}`); + } + + // `null`/empty files parse to `null`/`undefined` — treat as an empty object. + const input = parsed === null || parsed === undefined ? {} : parsed; + if (typeof input !== 'object' || Array.isArray(input)) { + throw new Error( + `agent.yaml at ${path} must be a YAML mapping, got ${Array.isArray(input) ? 'array' : typeof input}`, + ); + } + + const result = AgentConfigSchema.safeParse(input); + if (!result.success) { + throw new Error(`Invalid agent.yaml at ${path}: ${formatZodError(result.error)}`); + } + return result.data; +} + +/** Produce a concise, field-named error message from a ZodError. */ +function formatZodError(error: z.ZodError): string { + const issues = error.issues.map((issue) => { + const path = issue.path.length > 0 ? issue.path.join('.') : '(root)'; + // Zod v3 emits `unrecognized_keys` for strict() violations — surface the key names. + if (issue.code === 'unrecognized_keys') { + const keys = (issue as z.ZodIssue & { keys: string[] }).keys.join(', '); + return `unknown field(s) at ${path}: ${keys}`; + } + return `${path}: ${issue.message}`; + }); + return issues.join('; '); +} + +// ============================================================================ +// Writing +// ============================================================================ + +/** + * Serialize `config` to YAML and write it atomically to `path`. + * + * Derived fields (`name`, `dir`, `registeredAt`) are stripped before + * serialization so they never appear on disk. + * + * The write is protected by {@link acquireLock} and performed atomically + * (`${path}.tmp` + `rename`) so concurrent writers and concurrent readers + * never observe a partial/spliced file. + */ +export async function writeAgentYaml(path: string, config: AgentConfig): Promise { + const stripped = stripDerivedFields(config); + const yamlStr = yaml.dump(stripped, { + lineWidth: -1, + noRefs: true, + sortKeys: false, + quotingType: '"', + }); + + await acquireLock(path); + try { + const tmpPath = `${path}.tmp`; + await writeFile(tmpPath, yamlStr, 'utf-8'); + await rename(tmpPath, path); + } finally { + await releaseLock(path); + } +} + +// ============================================================================ +// Frontmatter extraction (pure helper — no I/O) +// ============================================================================ + +/** + * Split an AGENTS.md string into its leading YAML frontmatter block and body. + * + * Detects the `---\n...\n---\n` fence AT THE VERY START of the content (byte 0). + * Returns `{ frontmatter: , body: }` when found, else + * `{ frontmatter: null, body: content }`. + * + * Body preservation is byte-for-byte: CRLF endings, trailing newlines, and + * Unicode characters round-trip without modification. + * + * Pure function — no I/O, no mutation, safe to call from any context. + */ +export function extractFrontmatterFromAgentsMd(content: string): { frontmatter: string | null; body: string } { + // Must start at byte 0 with exactly `---\n` (no leading BOM, no leading + // whitespace). Without this anchor we'd accidentally match mid-file HR. + if (!content.startsWith('---\n')) { + return { frontmatter: null, body: content }; + } + + // Find the closing fence. A line that is exactly `---`: + // - when the frontmatter has content, the fence is preceded by `\n` → + // match `\n---`. + // - when the frontmatter is empty (`---\n---\n...`), the fence begins + // immediately after the opener at position 0 of `after`. + const after = content.slice(4); // skip the opening `---\n` + let frontmatter: string; + let afterClose: string; + + if (after.startsWith('---')) { + frontmatter = ''; + afterClose = after.slice(3); + } else { + const closeIdx = after.indexOf('\n---'); + if (closeIdx === -1) { + // Unclosed frontmatter — treat as "no frontmatter" rather than + // swallowing the entire file body. + return { frontmatter: null, body: content }; + } + frontmatter = after.slice(0, closeIdx); + afterClose = after.slice(closeIdx + 4); + } + + // A trailing `\n` after the closing fence is optional — when present we + // consume it so the body does not begin with a stray newline. + const body = afterClose.startsWith('\n') ? afterClose.slice(1) : afterClose; + + return { frontmatter, body }; +} diff --git a/src/lib/claude-native-teams.ts b/src/lib/claude-native-teams.ts index c3bd5be27..8e9b7fb1a 100644 --- a/src/lib/claude-native-teams.ts +++ b/src/lib/claude-native-teams.ts @@ -13,10 +13,11 @@ import { randomUUID } from 'node:crypto'; import { existsSync } from 'node:fs'; -import { mkdir, open, readFile, readdir, rm, stat, unlink, writeFile } from 'node:fs/promises'; +import { mkdir, open, readFile, readdir, rm, stat, writeFile } from 'node:fs/promises'; import { homedir } from 'node:os'; import { join } from 'node:path'; import { ensureTeammateBypassPermissions } from './claude-settings.js'; +import { acquireLock, releaseLock } from './lockfile.js'; import type { ClaudeTeamColor } from './provider-adapters.js'; import { CLAUDE_TEAM_COLORS } from './provider-adapters.js'; @@ -101,73 +102,6 @@ function inboxPath(teamName: string, agentName: string): string { return join(inboxesDir(teamName), `${sanitizeTeamName(agentName)}.json`); } -function lockPath(filePath: string): string { - return `${filePath}.lock`; -} - -// ============================================================================ -// Lockfile (simple polling lock for concurrent inbox writes) -// ============================================================================ - -const LOCK_TIMEOUT_MS = 5000; -const LOCK_POLL_MS = 50; - -/** Check if a PID is still alive using kill -0 (signal 0 = existence check). */ -function isPidAlive(pid: number): boolean { - try { - process.kill(pid, 0); - return true; - } catch { - return false; - } -} - -async function acquireLock(path: string): Promise { - const lock = lockPath(path); - const deadline = Date.now() + LOCK_TIMEOUT_MS; - - while (Date.now() < deadline) { - try { - await writeFile(lock, String(process.pid), { flag: 'wx' }); - return; // acquired - } catch { - // Lock exists — check if holder PID is still alive - try { - const content = await readFile(lock, 'utf-8'); - const holderPid = Number.parseInt(content.trim(), 10); - if (!Number.isNaN(holderPid) && !isPidAlive(holderPid)) { - // Holder is dead — remove stale lock and retry immediately - try { - await unlink(lock); - } catch { - // Another process may have already cleaned it up - } - continue; - } - } catch { - // Lock file disappeared between check and read — retry - continue; - } - - // Lock holder is alive — wait with jitter and retry - const jitter = Math.floor(Math.random() * LOCK_POLL_MS); - await new Promise((r) => setTimeout(r, LOCK_POLL_MS + jitter)); - } - } - - // Timeout — force acquire (likely stale lock) - console.warn(`[claude-native-teams] Force-acquiring stale lock: ${lock}`); - await writeFile(lock, String(process.pid)); -} - -async function releaseLock(path: string): Promise { - try { - await unlink(lockPath(path)); - } catch { - // Already released - } -} - // ============================================================================ // Config Operations // ============================================================================ diff --git a/src/lib/lockfile.ts b/src/lib/lockfile.ts new file mode 100644 index 000000000..bba2047e4 --- /dev/null +++ b/src/lib/lockfile.ts @@ -0,0 +1,104 @@ +/** + * Lockfile — Simple polling lock for concurrent file writes. + * + * PID-based lockfile with stale-lock detection and jittered retry. The holder + * writes its PID into `.lock`; waiters poll until the lock file is gone + * OR the holder PID is no longer alive (via `process.kill(pid, 0)`). + * + * Extracted from `claude-native-teams.ts` so any module that needs safe + * filesystem writes to a shared path can share the same lock semantics. + * + * Behavior: + * - `acquireLock(path)` creates `.lock` atomically (`wx` flag). + * - On conflict, reads the holder PID; if the holder is dead, removes the + * stale lock and retries immediately. + * - If the holder is alive, sleeps `LOCK_POLL_MS + jitter` before retrying. + * - After `LOCK_TIMEOUT_MS` elapses, force-acquires with a warning — safety + * valve for the pathological case where a holder hangs forever. + * - `releaseLock(path)` unlinks `.lock`; missing file is a no-op. + */ + +import { readFile, unlink, writeFile } from 'node:fs/promises'; + +// ============================================================================ +// Constants +// ============================================================================ + +const LOCK_TIMEOUT_MS = 5000; +const LOCK_POLL_MS = 50; + +// ============================================================================ +// Helpers +// ============================================================================ + +function lockPath(filePath: string): string { + return `${filePath}.lock`; +} + +/** Check if a PID is still alive using kill -0 (signal 0 = existence check). */ +function isPidAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +// ============================================================================ +// Public API +// ============================================================================ + +/** + * Acquire a lock on `.lock`. Blocks until the lock is available or the + * timeout elapses (in which case the lock is force-acquired with a warning). + * + * The caller must pair every successful `acquireLock` with a `releaseLock` in + * a `try { ... } finally { ... }` block. + */ +export async function acquireLock(path: string): Promise { + const lock = lockPath(path); + const deadline = Date.now() + LOCK_TIMEOUT_MS; + + while (Date.now() < deadline) { + try { + await writeFile(lock, String(process.pid), { flag: 'wx' }); + return; // acquired + } catch { + // Lock exists — check if holder PID is still alive + try { + const content = await readFile(lock, 'utf-8'); + const holderPid = Number.parseInt(content.trim(), 10); + if (!Number.isNaN(holderPid) && !isPidAlive(holderPid)) { + // Holder is dead — remove stale lock and retry immediately + try { + await unlink(lock); + } catch { + // Another process may have already cleaned it up + } + continue; + } + } catch { + // Lock file disappeared between check and read — retry + continue; + } + + // Lock holder is alive — wait with jitter and retry + const jitter = Math.floor(Math.random() * LOCK_POLL_MS); + await new Promise((r) => setTimeout(r, LOCK_POLL_MS + jitter)); + } + } + + // Timeout — force acquire (likely stale lock) + console.warn(`[lockfile] Force-acquiring stale lock: ${lock}`); + await writeFile(lock, String(process.pid)); +} + +/** Release a lock acquired via {@link acquireLock}. Missing file is a no-op. */ +export async function releaseLock(path: string): Promise { + try { + await unlink(lockPath(path)); + } catch { + // Already released + } +}