diff --git a/packages/daemon/src/daemon.ts b/packages/daemon/src/daemon.ts index 9a3bd493e0..921715119f 100644 --- a/packages/daemon/src/daemon.ts +++ b/packages/daemon/src/daemon.ts @@ -6229,14 +6229,55 @@ export class Daemon { ): Promise { const reported = plane?.workspaceIncarnationFor?.(pod) const shimGeneration = plane?.shimGenerationFor?.(pod) - return await this.reconcileSandboxSkills(agent, { + const isLaunchCurrent = () => + plane?.workspaceIncarnationFor?.(pod) === reported && plane?.shimGenerationFor?.(pod) === shimGeneration + const ledger = await this.reconcileSandboxSkills(agent, { client: plane?.skillClientFor?.(pod, cwd), // Receipts are relative to the directory installed into, so installs into a cwd keep a ledger of their own. workspaceIncarnation: reported && cwd ? cwdWorkspaceIncarnation(reported, cwd) : reported, shimGeneration, - isLaunchCurrent: () => - plane?.workspaceIncarnationFor?.(pod) === reported && plane?.shimGenerationFor?.(pod) === shimGeneration + isLaunchCurrent }) + if (cwd !== undefined && reported && shimGeneration !== undefined) { + await this.retireShimRootSkills(agent, plane?.skillClientFor?.(pod), reported, shimGeneration, isLaunchCurrent) + } + return ledger + } + + /** Remove the bundles an older daemon installed at a placed session's directory, above its checkout: no runtime scans there, and no other ledger owns them. */ + private async retireShimRootSkills( + agent: Agent, + client: ClusterSkillClient | undefined, + workspaceIncarnation: string, + shimGeneration: number, + isLaunchCurrent: () => boolean + ): Promise { + const prior = await this.store.clusterSkillLedger(agent.id, workspaceIncarnation) + if (!prior?.ledger.roots.length) return + const duty = this.duties.dutyForAgent(agent.id) + const skillsAgentId = this.runtimeCatalog.entries[agent.runtime]?.skillsAgentId + if (!client || !duty || !skillsAgentId || !this.cfg.daemonId) return + try { + await new ClusterSkillCoordinator(this.store).reconcile({ + authority: { + groupId: duty.groupId, + term: duty.term, + daemonId: this.cfg.daemonId, + agentId: agent.id, + workspaceIncarnation + }, + skillsAgentId, + shimGeneration, + sources: [], + client, + isLaunchCurrent + }) + } catch (error) { + // Retried at the next preparation: the ledger still holds them. + this.log.warn( + `skills: could not remove ${agent.id}'s bundles at its session directory (${(error as Error).message})` + ) + } } private async reconcileMicrosandboxSkills(agent: Agent, cwd: string): Promise { diff --git a/packages/daemon/src/shim/skill-client.ts b/packages/daemon/src/shim/skill-client.ts index 127657ab80..eb81321500 100644 --- a/packages/daemon/src/shim/skill-client.ts +++ b/packages/daemon/src/shim/skill-client.ts @@ -30,8 +30,7 @@ import { type ClusterSkillVerifyReply } from './skill-protocol.js' -/** A shim's skills seam aimed at `cwd` rather than the shim's own workspace root: the runtimes scan for project skills - * from their cwd up to its repository root only, so bundles beside a checkout rather than in it are never found. */ +/** A shim's skills seam aimed at `cwd`, not its own root: runtimes scan for project skills only from cwd up to the repository root. */ export function cwdSkillRequester(session: ShimRequester, cwd: string): ShimRequester { return { request: (capability, request, options) => session.request(capability, { cwd, request }, options) } } diff --git a/packages/daemon/test/daemon-lifecycle.test.ts b/packages/daemon/test/daemon-lifecycle.test.ts index 79eb8dec6d..ef18a36812 100644 --- a/packages/daemon/test/daemon-lifecycle.test.ts +++ b/packages/daemon/test/daemon-lifecycle.test.ts @@ -11,7 +11,9 @@ import { buildCpClientDeps } from '../src/cp/cp-client-deps.js' import { ExecutorPlane } from '../src/execution/executor-plane.js' import { TaskViolationError } from '../src/cp/task-reader.js' import { configFilesDir } from '../src/shim/config-file-env.js' +import { ClusterSkillCoordinator } from '../src/skills/cluster-skill-coordinator.js' import { readSkillLedger, skillLedgerLocation } from '../src/skills/skill-install-ledger.js' +import { cwdWorkspaceIncarnation } from '../src/skills/workspace-incarnation.js' import { sessionKey, transcriptChannelKey } from '../src/store/local-store.js' import { NO_RESPONSE_SENTINEL } from '../src/session/no-response.js' import { localWorkspaceFiles } from '../src/workspace/workspace-files.js' @@ -626,6 +628,84 @@ describe('Daemon session lifecycle (#118)', () => { } }) + it("installs a placed session's skills into its checkout and retires those at its session directory", async () => { + const daemon = new Daemon({ root: scaffold(), hostFactory: () => quietHost() as never }) + try { + await daemon.start() + const d = daemon as any + const agent = d.agents.get('bot-a') + d.runtimeCatalog.entries.claude = { ...d.runtimeCatalog.entries.claude, skillsAgentId: 'claude' } + const cwd = '/srv/executor/sessions/leaf/workspace' + const reported = 'workspace:reported' + const atCwd = cwdWorkspaceIncarnation(reported, cwd) + const root = (path: string) => ({ + path, + sourceId: 'managed:a', + sourceKind: 'managed' as const, + digest: 'a'.repeat(64), + files: [] + }) + const clients = new Map([ + [undefined, { manifestLimits: { maxFiles: 1, maxTotalBytes: 1 } }], + [cwd, { manifestLimits: { maxFiles: 1, maxTotalBytes: 1 } }] + ]) + d.executorPlane = { + placementOf: (key: string) => + key === KEY ? { agentId: 'bot-a', sessionKey: KEY, subject: 'bot-a/leaf' } : undefined, + withEnvironment: (_subject: string, work: () => Promise) => work(), + mountFor: () => '/srv/executor', + skillClientFor: (_subject: string, at?: string) => clients.get(at), + workspaceIncarnationFor: () => reported, + shimGenerationFor: () => 1, + planeFor: () => undefined, + launched: () => [], + releaseAgent: () => {}, + stop: async () => {} + } + vi.spyOn(d.workspaces, 'prepareExecutorWorkspace').mockResolvedValue(cwd) + const exclude = vi.spyOn(d.workspaces, 'excludePlacedSessionSkills').mockResolvedValue(undefined) + vi.spyOn(d.duties, 'dutyForAgent').mockReturnValue({ groupId: 'group-a', term: '1' }) + // An older daemon installed at the shim's root, the session directory; the checkout holds nothing yet. + const ledgers = new Map([[reported, { revision: 1, ledger: { roots: [root('.claude/skills/a')] } }]]) + vi.spyOn(d.store, 'clusterSkillLedger').mockImplementation((async (_id: string, key: string) => + ledgers.get(key)) as never) + let retirementFails = true + const reconcile = vi.spyOn(ClusterSkillCoordinator.prototype, 'reconcile').mockImplementation(async (input) => { + const at = input.authority.workspaceIncarnation + if (at === reported && retirementFails) { + retirementFails = false + throw new Error('shim busy') + } + const roots = at === atCwd ? [root('.claude/skills/a')] : [] + ledgers.set(at, { revision: 2, ledger: { roots } }) + return { roots } + }) + const request = { sessionKey: KEY, isolation: 'session' } + + // A failed retirement leaves the launch alone; the next preparation retries it. + expect(await d.runAgentWorkspacePreparation(agent, request)).toBe(cwd) + expect(reconcile.mock.calls.map(([input]) => [input.authority.workspaceIncarnation, input.client])).toEqual([ + [atCwd, clients.get(cwd)], + [reported, clients.get(undefined)] + ]) + expect(reconcile.mock.calls[1]![0].sources).toEqual([]) + expect(exclude).toHaveBeenCalledWith(agent, cwd, ['.claude/skills/a', '.agentconnect/cluster-skill-state']) + + reconcile.mockClear() + await d.runAgentWorkspacePreparation(agent, request) + expect(reconcile.mock.calls.map(([input]) => input.authority.workspaceIncarnation)).toEqual([atCwd, reported]) + expect(ledgers.get(reported)?.ledger.roots).toEqual([]) + + // Nothing is left there, so later preparations ask nothing of the session directory. + reconcile.mockClear() + await d.runAgentWorkspacePreparation(agent, request) + expect(reconcile.mock.calls.map(([input]) => input.authority.workspaceIncarnation)).toEqual([atCwd]) + } finally { + vi.restoreAllMocks() + await daemon.stop() + } + }) + it.each([true, false])('installs a host runtime only for local execution (runInSandbox=%s)', async (runInSandbox) => { const root = scaffold() const host = quietHost() diff --git a/packages/daemon/test/executor-plane.test.ts b/packages/daemon/test/executor-plane.test.ts index da0ca45303..755099f604 100644 --- a/packages/daemon/test/executor-plane.test.ts +++ b/packages/daemon/test/executor-plane.test.ts @@ -254,8 +254,7 @@ describe('the skills a placed session installs', () => { return { executor, request } } - // The shim is rooted at the session directory, above the checkout the runtime scans for project skills (Codex - // stops at the repository root), so installs are aimed at the runtime's cwd. + // The shim is rooted at the session directory, above the checkout where runtimes scan for project skills. it('aims every request at the cwd it is given', async () => { const { executor, request } = bound() await executor.skillClientFor(SUBJECT, CWD)!.verify([]) diff --git a/packages/daemon/test/git-exclude.test.ts b/packages/daemon/test/git-exclude.test.ts index e4b1bd3a57..131fbd7f3a 100644 --- a/packages/daemon/test/git-exclude.test.ts +++ b/packages/daemon/test/git-exclude.test.ts @@ -248,11 +248,18 @@ describe('the workspace manager recording the bundles its skills step installs', mkdir: async () => {}, writeFile: async (path: string, content: string) => void written.push({ path, content }) } + const unscoped = { + readFileBytes: async () => { + throw new Error('asked without the path that names the session') + } + } const workspaces = new WorkspaceManager() wireTestPlane(workspaces, { workspacesOffDisk: true, gitRunnerFor: () => runner, - workspaceFsFor: () => ({ fs, mount: '/srv/executor' }) as never + // A placed session's filesystem is found by its path; an agent-wide question would reach the holder's own disk. + workspaceFsFor: (_agentId, scope) => + ({ fs: scope?.path === cwd ? fs : unscoped, mount: '/srv/executor' }) as never }) await workspaces.excludePlacedSessionSkills(agent, cwd, [BUNDLE, '.agentconnect/cluster-skill-state']) // The exclude file is the checkout's own, on the executor: nothing is written on this disk.