fix(agent-presets,web): broken presets are roster rows, not gaps
A hand-damaged preset was silent until the worst moment. An unparsable composition listed as an ordinary selectable row and failed only at the next session start — set as default, every new session failed. A directory whose composition file was deleted vanished from the roster while still occupying its id: copy answered "delete the existing preset first" while remove answered "not found", a dead end. Discovery now owns health: every id-shaped directory is a roster slot, broken when its composition is missing or unloadable, checked with the loader's own entryListSchema dialect (!!js included) so health never rejects what the loader accepts. `broken` rides AgentPreset, the agentPreset.list entry, and the UI row; mount/recompose/standingKeyFor refuse broken up front with the discovery-reported reason, while resolve/read/remove still answer. The section renders marked red cards — unselectable, uncopyable, deletable, location kept on custom rows — and both pickers drop broken rows entirely. The cordis preset's persona now forbids editing the shipped install (corrupting cordis would disable the mode itself) and points authoring at $DSH_HOME/.agent-presets; its skill teaches preset.yml metadata, the copy-first workflow, the one-escalation sandbox reality, and honest verification. Exercised live: asked to edit the shipped composition the composed agent refuses citing both rules; asked for real presets (simple and complex) it lands them under the user root with one approved escalation each and self-checks with the loader dialect.
This commit is contained in:
@@ -17,16 +17,7 @@ import { dirname, isAbsolute, join, resolve } from 'node:path'
|
||||
import { writeFileAtomic } from '@deepseek-ai/dsh-atomic-write'
|
||||
import { expandHomePath } from '@deepseek-ai/dsh-paths'
|
||||
import { METADATA_FILE, renderPresetMetadata } from './metadata.ts'
|
||||
import type { AgentPreset, PresetRoot } from './types.ts'
|
||||
|
||||
/**
|
||||
* Ids a preset directory may use.
|
||||
*
|
||||
* The id becomes a path segment, so this is a containment boundary rather than
|
||||
* a style rule: `..`, a separator, or an absolute-looking name would place the
|
||||
* composition outside the root the deployment authorised.
|
||||
*/
|
||||
const PRESET_ID = /^[a-z0-9][a-z0-9-]*$/
|
||||
import { PRESET_ID, type AgentPreset, type PresetRoot } from './types.ts'
|
||||
|
||||
/** A preset id that cannot be used as a directory name under a root. */
|
||||
export class InvalidPresetIdError extends Error {
|
||||
|
||||
@@ -4,18 +4,92 @@
|
||||
* its display text; the directory name is the preset id. Discovery
|
||||
* re-reads the roots on every call so a preset authored while the process is
|
||||
* running is visible without a restart.
|
||||
*
|
||||
* Discovery also owns preset HEALTH: a directory whose composition is
|
||||
* missing or unloadable is reported as a broken roster row rather than
|
||||
* skipped. A skipped directory would still occupy its id on disk — the copy
|
||||
* path refuses the name while no surface shows anything to delete — and a
|
||||
* malformed composition would otherwise read as an ordinary preset until the
|
||||
* first session fails to mount it.
|
||||
* @module @deepseek-ai/dsh-agent-presets/discovery
|
||||
*/
|
||||
|
||||
import { readdir, stat } from 'node:fs/promises'
|
||||
import { readdir, readFile, stat } from 'node:fs/promises'
|
||||
import { join, resolve } from 'node:path'
|
||||
import { load } from 'js-yaml'
|
||||
import { entryListSchema } from '@cordisjs/plugin-include'
|
||||
import { expandHomePath } from '@deepseek-ai/dsh-paths'
|
||||
import { readPresetMetadata } from './metadata.ts'
|
||||
import type { AgentPreset, PresetRoot } from './types.ts'
|
||||
import { PRESET_ID, type AgentPreset, type PresetRoot } from './types.ts'
|
||||
|
||||
/** The composition file that makes a directory a preset. */
|
||||
export const COMPOSITION_FILE = 'agent.cordis.yml'
|
||||
|
||||
/**
|
||||
* Why `rows` cannot be an entry list, or undefined when it can.
|
||||
*
|
||||
* A shallow shape check, deliberately short of the loader's work: it does not
|
||||
* resolve plugin names or apply configs. What it catches is the hand-edit
|
||||
* that produces a file the loader cannot even begin with — and it must accept
|
||||
* everything the loader accepts, which is why rows are only required to be
|
||||
* maps carrying a plugin `name` (groups recurse into their own lists).
|
||||
* @param rows - the parsed composition document.
|
||||
* @param at - row-path prefix for nested diagnostics, empty at the top level.
|
||||
* @returns one human-readable reason, or undefined when the shape holds.
|
||||
*/
|
||||
function entryListProblem(rows: unknown, at = ''): string | undefined {
|
||||
if (!Array.isArray(rows)) {
|
||||
return at === ''
|
||||
? 'the composition must be a top-level list of plugin rows'
|
||||
: `group ${at} must hold a list of plugin rows`
|
||||
}
|
||||
for (const [index, row] of rows.entries()) {
|
||||
const label = at === '' ? `row ${String(index + 1)}` : `${at} row ${String(index + 1)}`
|
||||
if (typeof row !== 'object' || row === null || Array.isArray(row)) {
|
||||
return `${label} is not a plugin row (expected a map with a "name")`
|
||||
}
|
||||
const { name, group, config } = row as { name?: unknown; group?: unknown; config?: unknown }
|
||||
if (typeof name !== 'string' || name === '') {
|
||||
return `${label} names no plugin (a "name" string is required)`
|
||||
}
|
||||
if (group === true) {
|
||||
const nested = entryListProblem(config, label)
|
||||
if (nested !== undefined) return nested
|
||||
}
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Why the composition at `path` cannot mount, or undefined when it looks
|
||||
* loadable. Parsed with the loader's own YAML dialect ({@link entryListSchema},
|
||||
* the one carrying `!!js`), so health can never call a composition broken
|
||||
* that the loader would accept.
|
||||
* @param path - absolute path of the composition file.
|
||||
* @returns one human-readable reason, or undefined when the file is loadable.
|
||||
*/
|
||||
async function compositionProblem(path: string): Promise<string | undefined> {
|
||||
let content: string
|
||||
try {
|
||||
content = await readFile(path, 'utf8')
|
||||
} catch {
|
||||
// The caller statted this file moments ago; any read failure now —
|
||||
// deleted in between, permissions — is the same answer as unparsable.
|
||||
return `the composition file ${COMPOSITION_FILE} cannot be read`
|
||||
}
|
||||
let rows: unknown
|
||||
try {
|
||||
rows = load(content, { schema: entryListSchema })
|
||||
} catch (error) {
|
||||
/* v8 ignore next -- js-yaml throws YAMLException (an Error) for every parse failure; the fallback keeps a hostile value readable */
|
||||
const full = error instanceof Error ? error.message : String(error)
|
||||
// First line only: js-yaml appends a multi-line code-frame snippet, and
|
||||
// the reason is displayed on a roster card, not in a terminal.
|
||||
return `the composition is not valid YAML: ${full.replace(/\n[\s\S]*$/, '')}`
|
||||
}
|
||||
return entryListProblem(rows)
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `path` names an existing regular file.
|
||||
* @param path - absolute path to test.
|
||||
@@ -38,6 +112,12 @@ async function isFile(path: string): Promise<boolean> {
|
||||
* An absent root yields no presets rather than throwing: the user root does
|
||||
* not exist until the first locally authored preset, and naming a default
|
||||
* that no root supplies already fails loud at resolution.
|
||||
*
|
||||
* Every directory whose name is a usable preset id is a roster row — broken
|
||||
* when its composition is missing or unloadable. A directory named outside
|
||||
* {@link PRESET_ID} is skipped instead: no copy could ever claim that name,
|
||||
* so it blocks nothing, and reporting `.DS_Store`-grade residue as broken
|
||||
* presets would teach users to ignore the marker.
|
||||
* @param root - the directory and the trust its presets inherit.
|
||||
* @returns the root's presets ordered by id.
|
||||
*/
|
||||
@@ -52,14 +132,19 @@ export async function scanRoot(root: PresetRoot): Promise<AgentPreset[]> {
|
||||
}
|
||||
const found: AgentPreset[] = []
|
||||
for (const child of children) {
|
||||
if (!child.isDirectory()) continue
|
||||
if (!child.isDirectory() || !PRESET_ID.test(child.name)) continue
|
||||
const directory = join(dir, child.name)
|
||||
const path = join(directory, COMPOSITION_FILE)
|
||||
if (!await isFile(path)) continue
|
||||
const broken = await isFile(path)
|
||||
? await compositionProblem(path)
|
||||
: `the composition file ${COMPOSITION_FILE} is missing — the directory still occupies the id; delete it or restore the file`
|
||||
// Display text only, and never fatal: a preset with unreadable metadata
|
||||
// still mounts, it just shows its id.
|
||||
const metadata = await readPresetMetadata(directory)
|
||||
found.push({ id: child.name, trust: root.trust, path, ...metadata })
|
||||
found.push({
|
||||
id: child.name, trust: root.trust, path, ...metadata,
|
||||
...broken === undefined ? {} : { broken },
|
||||
})
|
||||
}
|
||||
// Declared order first so the shipped set reads by capability; everything
|
||||
// else falls back to the id, which keeps authored presets stable.
|
||||
|
||||
@@ -153,6 +153,10 @@ export class AgentPresets extends Service {
|
||||
|
||||
/**
|
||||
* Resolve one preset by id.
|
||||
*
|
||||
* A broken preset resolves — deleting one, reading one, and reporting one
|
||||
* all need the row — and the mounting paths refuse it AFTER resolution
|
||||
* through {@link resolveMountable}.
|
||||
* @param id - the preset id, or `undefined` for {@link defaultId}.
|
||||
* @returns the resolved preset.
|
||||
* @throws when no configured root supplies that id.
|
||||
@@ -167,6 +171,24 @@ export class AgentPresets extends Service {
|
||||
return found
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve one preset that is about to compose an agent, refusing a broken
|
||||
* one with its discovery-reported reason. Failing here rather than inside
|
||||
* the loader keeps the answer the same for every unloadable shape — ghost
|
||||
* directory, unparsable YAML, rowless list — and spends no mount attempt
|
||||
* on a composition discovery already read as unusable.
|
||||
* @param id - the preset id, or `undefined` for {@link defaultId}.
|
||||
* @returns the resolved, mountable preset.
|
||||
* @throws when the preset is unknown or discovery reports it broken.
|
||||
*/
|
||||
private async resolveMountable(id?: string): Promise<AgentPreset> {
|
||||
const preset = await this.resolve(id)
|
||||
if (preset.broken !== undefined) {
|
||||
throw new PresetMountError(preset.id, preset.broken)
|
||||
}
|
||||
return preset
|
||||
}
|
||||
|
||||
/**
|
||||
* Standing mounts by preset id, single-flight so two agents racing the
|
||||
* first use of one preset share one composition. A settled failure is
|
||||
@@ -198,7 +220,7 @@ export class AgentPresets extends Service {
|
||||
if (agentKey === undefined) {
|
||||
throw new Error('agent-presets: refusing to compose an unscoped context; the scope key is what joins an agent to its preset')
|
||||
}
|
||||
const preset = await this.resolve(id)
|
||||
const preset = await this.resolveMountable(id)
|
||||
const standing = await this.ensureStanding(preset)
|
||||
setScopeParent(agentKey, standing.key)
|
||||
return preset
|
||||
@@ -314,7 +336,7 @@ export class AgentPresets extends Service {
|
||||
if (agentKey === undefined) {
|
||||
throw new Error('agent-presets: refusing to recompose an unscoped context')
|
||||
}
|
||||
const preset = await this.resolve(id)
|
||||
const preset = await this.resolveMountable(id)
|
||||
const standing = await this.ensureStanding(preset)
|
||||
setScopeParent(agentKey, standing.key)
|
||||
return preset
|
||||
@@ -332,7 +354,7 @@ export class AgentPresets extends Service {
|
||||
* @throws when the preset is unknown or its composition is unusable.
|
||||
*/
|
||||
async standingKeyFor(id?: string): Promise<ScopeKey> {
|
||||
const preset = await this.resolve(id)
|
||||
const preset = await this.resolveMountable(id)
|
||||
return (await this.ensureStanding(preset)).key
|
||||
}
|
||||
|
||||
|
||||
@@ -7,6 +7,16 @@
|
||||
*/
|
||||
export type PresetTrust = 'system' | 'user'
|
||||
|
||||
/**
|
||||
* Ids a preset directory may use.
|
||||
*
|
||||
* The id becomes a path segment, so this is a containment boundary rather than
|
||||
* a style rule: `..`, a separator, or an absolute-looking name would place the
|
||||
* composition outside the root the deployment authorised. Discovery shares it:
|
||||
* a directory whose name no copy could ever claim is not a preset slot.
|
||||
*/
|
||||
export const PRESET_ID = /^[a-z0-9][a-z0-9-]*$/
|
||||
|
||||
/** One preset directory that carries a mountable agent composition. */
|
||||
export interface AgentPreset {
|
||||
/** Stable identifier; the preset directory's name. */
|
||||
@@ -21,6 +31,13 @@ export interface AgentPreset {
|
||||
readonly description?: string
|
||||
/** Declared position within its group; absent sorts after those that declare one. */
|
||||
readonly order?: number
|
||||
/**
|
||||
* Why this preset cannot compose a session, absent when it can. A broken
|
||||
* preset stays on the roster — hiding it would leave its directory blocking
|
||||
* the id with nothing to see or delete — but every mounting path refuses it
|
||||
* up front with this reason instead of failing deep inside the loader.
|
||||
*/
|
||||
readonly broken?: string
|
||||
}
|
||||
|
||||
/** One directory scanned for preset subdirectories. */
|
||||
|
||||
Reference in New Issue
Block a user