feat(agent-presets): own the writable preset root instead of awaiting an app

`Config.roots` defaulted to `[]`, so a deployment that did not patch both roots
in got a roster with none — `dsh run` once booted exactly that and failed
resolving `standard`. The user root never needed an app: `<dshHome>/.agent-presets`
is the same place in every deployment, resolvable here the way
`dsh-skill-local` resolves `<dshHome>/skills`.

The roster now derives that root itself unless `includeUserRoot` is false, and
`apps/cli` supplies only the SHIPPED root, whose path an installed app alone
can resolve. The derived root is appended after every configured root, so a
shipped id still shadows a home directory claiming it and `writableRoot()`
still prefers an explicitly configured `user` root; the set is resolved once at
construction, because a root set that changed between a `list()` and the
`copy()` acting on its answer would author into a directory the caller never
saw.

Every test that pins an exact roster now says `includeUserRoot: false` — the
machine's real harness home must not decide what a golden or an assertion
counts.
This commit is contained in:
Yichen Jiang
2026-08-11 20:20:16 +08:00
parent ac266ed2de
commit 5c7dd6f8eb
21 changed files with 253 additions and 38 deletions
@@ -52,6 +52,10 @@ beforeEach(async () => {
{ path: join(FIXTURES, 'system'), trust: 'system' as const },
{ path: userRoot, trust: 'user' as const },
],
// Every roster in this file pins its own roots: the derived harness-home
// root would add the developer's real presets to what these assertions
// count, and `copy` would write into it.
includeUserRoot: false,
})
})
@@ -199,6 +203,7 @@ describe('a deployment with more than one user root', () => {
{ path: userRoot, trust: 'user' as const },
{ path: second, trust: 'user' as const },
],
includeUserRoot: false,
})
// Writes go to the first user root, so a preset discovered from a later
@@ -219,6 +224,7 @@ describe('a deployment with no writable root', () => {
await readOnly.plugin(AgentPresets, {
default: 'standard',
roots: [{ path: join(FIXTURES, 'system'), trust: 'system' as const }],
includeUserRoot: false,
})
expect(readOnly.agentPresets.authorable).toBe(false)
@@ -240,6 +246,7 @@ describe('a user root that does not exist yet', () => {
{ path: join(FIXTURES, 'system'), trust: 'system' as const },
{ path: absent, trust: 'user' as const },
],
includeUserRoot: false,
})
await fresh.agentPresets.copy('standard', 'mine')
@@ -31,7 +31,7 @@ async function harness(): Promise<Context> {
await ctx.plugin(ToolRegistry)
await ctx.plugin(AgentRegistry)
await ctx.plugin(AgentLoop, { agents: [] })
await ctx.plugin(AgentPresets, { default: 'standard', roots: ROOTS })
await ctx.plugin(AgentPresets, { default: 'standard', roots: ROOTS, includeUserRoot: false })
await ctx.plugin(InvariantService)
await ctx.plugin(AgentPresetsInvariant)
return ctx
@@ -38,7 +38,7 @@ const ROOTS = [
* @param roster - roster config, defaulting to the fixture roots.
* @returns the booted context.
*/
async function harness(roster: Config = { default: 'standard', roots: ROOTS }): Promise<Context> {
async function harness(roster: Config = { default: 'standard', roots: ROOTS, includeUserRoot: false }): Promise<Context> {
const ctx = new Context()
ctx.baseUrl = pathToFileURL(FIXTURES).href + '/'
await ctx.plugin(Loader)
@@ -94,7 +94,7 @@ describe('composing an agent from a preset', () => {
join(presetDir, COMPOSITION_FILE),
`- id: only\n name: ${plugin}\n config:\n tool: absolute\n`,
)
const scoped = await harness({ default: 'absolute', roots: [{ path: root, trust: 'user' }] })
const scoped = await harness({ default: 'absolute', roots: [{ path: root, trust: 'user' }], includeUserRoot: false })
const imported = vi.spyOn(scoped.loader.internal!, 'import')
await agentOn(scoped, 'sess-absolute-plugin')
@@ -347,7 +347,7 @@ describe('composing from a broken preset', () => {
const root = await mkdtemp(join(tmpdir(), 'dsh-preset-broken-'))
await mkdir(join(root, 'damaged'))
await writeFile(join(root, 'damaged', COMPOSITION_FILE), composition)
return await harness({ default: 'damaged', roots: [{ path: root, trust: 'user' as const }] })
return await harness({ default: 'damaged', roots: [{ path: root, trust: 'user' as const }], includeUserRoot: false })
}
it('refuses the mount up front with the discovery-reported reason', async () => {
@@ -380,7 +380,7 @@ describe('a roster with nothing in it', () => {
it('says so instead of naming an empty list of candidates', async () => {
const bare = new Context()
await bare.plugin(Loader)
await bare.plugin(AgentPresets, { default: 'standard', roots: [] })
await bare.plugin(AgentPresets, { default: 'standard', roots: [], includeUserRoot: false })
await expect(bare.agentPresets.resolve())
.rejects.toThrow(/preset "standard" not found \(available: none\)/)
@@ -418,7 +418,7 @@ describe('the preset file is an input, never a persistence target', () => {
await scoped.plugin(ToolRegistry)
await scoped.plugin(AgentRegistry)
await scoped.plugin(AgentLoop, { agents: [] })
await scoped.plugin(AgentPresets, { default: 'self-disposing', roots: [{ path: root, trust: 'user' as const }] })
await scoped.plugin(AgentPresets, { default: 'self-disposing', roots: [{ path: root, trust: 'user' as const }], includeUserRoot: false })
await scoped.agents.create({
sessionId: SessionId('sess-self-dispose'),
@@ -532,7 +532,7 @@ describe('replacing a composition', () => {
// Presets are optional: every surface except the Web bundle keeps its
// model-facing rows in the host plane, so an agent with a chain of one is
// exactly right there and the diagnostic must stay silent.
const rosterless = await harness({ default: 'standard', roots: [] })
const rosterless = await harness({ default: 'standard', roots: [], includeUserRoot: false })
const warnings: string[] = []
rosterless.logger.warn = ((message: unknown) => { warnings.push(String(message)) }) as typeof rosterless.logger.warn
@@ -581,7 +581,7 @@ describe('replacing a composition', () => {
await scoped.plugin(ToolRegistry)
await scoped.plugin(AgentRegistry)
await scoped.plugin(AgentLoop, { agents: [] })
await scoped.plugin(AgentPresets, { default: 'first', roots: [{ path: root, trust: 'user' as const }] })
await scoped.plugin(AgentPresets, { default: 'first', roots: [{ path: root, trust: 'user' as const }], includeUserRoot: false })
const handle = await scoped.agents.create({
sessionId: SessionId('sess-restore-gone'),
setup: async (agentCtx: Context) => void await scoped.agentPresets.mount(agentCtx, 'first'),
@@ -621,7 +621,7 @@ describe('editing a composition file', () => {
await mkdir(join(root, id))
const path = join(root, id, COMPOSITION_FILE)
await writeFile(path, rowFor('before'))
const scoped = await harness({ default: id, roots: [{ path: root, trust: 'user' as const }] })
const scoped = await harness({ default: id, roots: [{ path: root, trust: 'user' as const }], includeUserRoot: false })
return { scoped, path }
}
@@ -49,7 +49,7 @@ async function harness(
await ctx.plugin(AgentLoop, { agents: [] })
const settingsFiber = ctx.plugin(SettingsLocal, { path: settingsFile, watch: false })
await settingsFiber
await ctx.plugin(AgentPresets, { default: 'standard', roots: [...ROOTS, ...extraRoots] })
await ctx.plugin(AgentPresets, { default: 'standard', roots: [...ROOTS, ...extraRoots], includeUserRoot: false })
return { ctx, settingsFile, settingsFiber }
}
@@ -0,0 +1,129 @@
/**
* The writable root is this package's own, not an assembly fact each app must
* remember: a roster configured with only a `system` root still discovers and
* authors into `<dshHome>/.agent-presets`, the way `dsh-skill-local` owns
* `<dshHome>/skills`. `includeUserRoot: false` is how a deployment — or a test
* pinning an exact roster — opts out.
*
* `$DSH_HOME` is repointed per test because the derived root is resolved in the
* constructor: the plugin must be mounted while the environment names the
* temporary home, or it would reach the developer's real one.
*/
import { mkdtemp, mkdir, writeFile } from 'node:fs/promises'
import { existsSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { dirname, join } from 'node:path'
import { fileURLToPath, pathToFileURL } from 'node:url'
import { Context } from '@deepseek-ai/cordis'
import Loader from '@deepseek-ai/cordis-plugin-loader'
import Include from '@deepseek-ai/cordis-plugin-include'
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import AgentPresets, { COMPOSITION_FILE, USER_PRESET_DIR, type Config } from '@deepseek-ai/dsh-agent-presets'
const FIXTURES = join(dirname(fileURLToPath(import.meta.url)), 'fixtures')
const SYSTEM_ROOT = join(FIXTURES, 'system')
const VALID = '- id: tool-alpha\n name: ../../plugins/contribute.js\n config:\n tool: alpha\n'
let home: string
let previousHome: string | undefined
beforeEach(async () => {
home = await mkdtemp(join(tmpdir(), 'dsh-preset-home-'))
previousHome = process.env.DSH_HOME
process.env.DSH_HOME = home
})
afterEach(() => {
if (previousHome === undefined) delete process.env.DSH_HOME
else process.env.DSH_HOME = previousHome
})
/** Boot a roster over the fixture system root, with the derived root left to the plugin. */
async function roster(config: Partial<Config> = {}): Promise<Context> {
const ctx = new Context()
ctx.baseUrl = pathToFileURL(FIXTURES).href + '/'
await ctx.plugin(Loader)
ctx.loader.builtins.include = Include
await ctx.plugin(AgentPresets, {
default: 'standard',
roots: [{ path: SYSTEM_ROOT, trust: 'system' as const }],
includeUserRoot: true,
...config,
})
return ctx
}
/** Hand-place a preset directory under the harness home's preset root. */
async function seedHomePreset(id: string): Promise<void> {
await mkdir(join(home, USER_PRESET_DIR, id), { recursive: true })
await writeFile(join(home, USER_PRESET_DIR, id, COMPOSITION_FILE), VALID)
}
describe('the harness-home preset root', () => {
it('is what a roster gets when config names no roots at all', () => {
// The schema default is the contract an app relies on by saying nothing;
// every other case here passes the field explicitly. The cast stands for
// the untyped document the Loader hands the schema, which is where a
// composition that omits the key actually comes from.
const parsed = AgentPresets.Config({ default: 'standard' } as unknown as Config)
expect(parsed).toMatchObject({ includeUserRoot: true, roots: [] })
})
it('is discovered without any app configuring it', async () => {
await seedHomePreset('mine')
const ctx = await roster()
const listed = await ctx.agentPresets.list()
expect(listed.find(preset => preset.id === 'mine')).toMatchObject({ trust: 'user' })
expect((await ctx.agentPresets.resolve('mine')).path)
.toBe(join(home, USER_PRESET_DIR, 'mine', COMPOSITION_FILE))
})
it('makes a roster with only a system root authorable, and receives the copy', async () => {
const ctx = await roster()
expect(ctx.agentPresets.authorable).toBe(true)
await ctx.agentPresets.copy('standard', 'copied')
expect(existsSync(join(home, USER_PRESET_DIR, 'copied', COMPOSITION_FILE))).toBe(true)
})
it('sorts after every configured root, so a shipped id still shadows a home directory', async () => {
// `standard` exists in the fixture system root; claiming the name at home
// must not take it over, because `copy` refuses an id any root supplies
// and a session resolving `standard` must reach the shipped composition.
await seedHomePreset('standard')
const ctx = await roster()
expect((await ctx.agentPresets.resolve('standard')).trust).toBe('system')
await expect(ctx.agentPresets.copy('standard', 'standard')).rejects.toThrow(/already exists/)
})
it('is absent under includeUserRoot: false, which leaves the roster unauthorable', async () => {
await seedHomePreset('mine')
const ctx = await roster({ includeUserRoot: false })
expect((await ctx.agentPresets.list()).map(preset => preset.id)).not.toContain('mine')
expect(ctx.agentPresets.authorable).toBe(false)
await expect(ctx.agentPresets.copy('standard', 'mine'))
.rejects.toThrow(/no user-writable preset root/)
})
it('yields to a configured user root for authoring, which writableRoot takes first', async () => {
const explicit = await mkdtemp(join(tmpdir(), 'dsh-preset-explicit-'))
const ctx = await roster({
roots: [
{ path: SYSTEM_ROOT, trust: 'system' as const },
{ path: explicit, trust: 'user' as const },
],
})
await ctx.agentPresets.copy('standard', 'copied')
expect(existsSync(join(explicit, 'copied', COMPOSITION_FILE))).toBe(true)
expect(existsSync(join(home, USER_PRESET_DIR, 'copied'))).toBe(false)
})
})