Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions src/agent/hosted/chat-preparation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -635,3 +635,69 @@ Deno.test("prepareHostedChatRuntimeMessages omits provider-owned remote tool his
text: "Unlimited tax liability is based on Chapter 3 of the Income Tax Act.",
}]);
});

Deno.test("prepareHostedChatRuntimeCreationOptions filters skills to the run agent's owner scope", async () => {
const skills = [
{
id: "global-howto",
name: "Global Howto",
description: "Project-global guide",
instructions: "Follow the guide.",
allowedTools: [],
},
{
id: "researcher--cite",
name: "cite",
description: "Cite sources",
instructions: "Cite primary sources.",
allowedTools: [],
ownerAgentId: "researcher",
shortName: "cite",
sourcePath: "agents/researcher/skills/cite/SKILL.md",
},
{
id: "writer--style",
name: "style",
description: "House style",
instructions: "Use the house style.",
allowedTools: [],
ownerAgentId: "writer",
shortName: "style",
},
];
const seenByInstructions: string[][] = [];

const result = await prepareHostedChatRuntimeCreationOptions({
request: createParsedHostedChatRequest({}),
agentConfig: { id: "researcher", model: "configured-model" },
projectId: "project-1",
authToken: "token-1",
resolveModelId: (modelId) => modelId,
resolveModelThinking: () => undefined,
fetchSteering: () =>
Promise.resolve({
instructions: "Project instructions",
skills,
}),
buildInstructions: (input) => {
seenByInstructions.push(input.skills.map((entry) => entry.id));
return [{ role: "system", content: "x" }];
},
});

const expected = ["global-howto", "researcher--cite"];
// Prompt-manifest input, per-run load_skill gate, live steering payload, and
// the returned steering all carry the same owner-scoped set.
assertEquals(seenByInstructions, [expected]);
assertEquals(result.creationOptions.availableSkillIds, expected);
assertEquals(result.creationOptions.skillSourcePaths, {
"researcher--cite": "agents/researcher/skills/cite/SKILL.md",
});
assertEquals(
(result.creationOptions.liveProjectSteering?.initialSkills ?? []).map((
skill: { id: string },
) => skill.id),
expected,
);
assertEquals(result.steering.skills.map((skill) => skill.id), expected);
});
25 changes: 21 additions & 4 deletions src/agent/hosted/chat-preparation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
resolveHostedRuntimeRequestConfig,
} from "./runtime-request-config.ts";
import { getRuntimeUploadUrl } from "../runtime/upload-url-client.ts";
import type { RuntimeSkillDefinition } from "../runtime/skill-metadata.ts";
import { isRuntimeSkillVisibleTo, type RuntimeSkillDefinition } from "../runtime/skill-metadata.ts";
import {
applyContextBudget,
type ContextBudgetDiagnostics,
Expand Down Expand Up @@ -266,13 +266,20 @@ export async function prepareHostedChatRuntimeCreationOptions<
authToken: input.authToken,
branchId: input.branchId,
});
// Owner-aware: the run only ever sees skills visible to its agent (unowned
// plus the agent's own). Filtering here scopes the prompt manifest, the
// per-run availableSkillIds gate used by hosted load_skill, its not-found
// enumeration, and the live steering payload in one place.
const visibleSkills = steering.skills.filter((skill) =>
isRuntimeSkillVisibleTo(skill, { agentId: input.agentConfig.id })
);
const agentInstructions = input.buildInstructions({
agentConfig: input.agentConfig,
projectId: input.projectId,
branchId: input.branchId,
environmentContext: input.environmentContext,
instructions: steering.instructions,
skills: steering.skills,
skills: visibleSkills,
});
const runtimeConfig = resolveHostedRuntimeRequestConfig({
request: input.request,
Expand Down Expand Up @@ -312,7 +319,16 @@ export async function prepareHostedChatRuntimeCreationOptions<
...(input.rootRunContext?.effectiveParentMessageId
? { parentMessageId: input.rootRunContext.effectiveParentMessageId }
: {}),
availableSkillIds: steering.skills.map((skill) => skill.id),
availableSkillIds: visibleSkills.map((skill) => skill.id),
...(visibleSkills.some((skill) => skill.sourcePath)
? {
skillSourcePaths: Object.fromEntries(
visibleSkills
.filter((skill) => skill.sourcePath)
.map((skill) => [skill.id, skill.sourcePath as string]),
),
}
: {}),
...(input.rootRunContext?.publishParentRunEvents
? { publishParentRunEvents: input.rootRunContext.publishParentRunEvents }
: {}),
Expand All @@ -321,11 +337,12 @@ export async function prepareHostedChatRuntimeCreationOptions<
agentConfig: input.agentConfig,
environmentContext: input.environmentContext,
instructions: steering.instructions,
skills: steering.skills,
skills: visibleSkills,
}),
},
steering: {
...steering,
skills: visibleSkills,
agentInstructions,
},
runtimeConfig,
Expand Down
2 changes: 2 additions & 0 deletions src/agent/hosted/chat-runtime-contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ export type HostedChatRuntimeCreationOptions<TRuntimeAgentDefinition, TThinkingC
parentRunId?: string;
parentMessageId?: string;
availableSkillIds?: string[];
/** Per-run skill id -> discovered SKILL.md source path (owner-aware catalog). */
skillSourcePaths?: Readonly<Record<string, string>>;
publishParentRunEvents?: (events: ConversationRunEvent[]) => Promise<void>;
clientProfile?: RuntimeClientProfile | null;
liveProjectSteering?: HostedChatRuntimeProjectSteering<TRuntimeAgentDefinition>;
Expand Down
3 changes: 3 additions & 0 deletions src/agent/hosted/default-chat-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ export type DefaultHostedChatRuntimeTaskContext = HostedRuntimeStateResolverCont
parentRunId?: string;
parentMessageId?: string;
availableSkillIds?: string[];
/** Per-run skill id -> discovered SKILL.md source path (owner-aware catalog). */
skillSourcePaths?: Readonly<Record<string, string>>;
publishParentRunEvents?: DefaultHostedChatRuntimeCreationOptions["publishParentRunEvents"];
availableToolNames?: string[];
};
Expand Down Expand Up @@ -149,6 +151,7 @@ function createDefaultTaskContext(
parentRunId: input.options.parentRunId,
parentMessageId: input.options.parentMessageId,
availableSkillIds: input.options.availableSkillIds,
skillSourcePaths: input.options.skillSourcePaths,
publishParentRunEvents: input.options.publishParentRunEvents,
};
}
Expand Down
57 changes: 57 additions & 0 deletions src/agent/hosted/project-steering-adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,60 @@ Deno.test("hosted project steering adapter accepts a custom builtin skill store"
assertEquals(referenceCalls, [{ skillsDir, skillId: "custom" }]);
});
});

Deno.test("refreshProjectSkillIds keeps the caller's owner scope and source paths", async () => {
await withSkillsDir(async (skillsDir) => {
const skillMd = (name: string) =>
`---
name: ${name}
description: ${name} skill
---
Body.`;
const adapter = createHostedProjectSteeringAdapter({
apiUrl: "https://api.example.test",
skillsDir,
projectFilesClient: createProjectFilesClient({
getProjectFile: async ({ path }) =>
path === "skills/global/SKILL.md" ||
path === "agents/researcher/skills/cite/SKILL.md" ||
path === "agents/writer/skills/style/SKILL.md"
? { path, content: skillMd(path) }
: null,
getProjectFiles: async () => [
{ path: "skills/global/SKILL.md" },
{ path: "agents/researcher/skills/cite/SKILL.md" },
{ path: "agents/writer/skills/style/SKILL.md" },
],
}),
});

// With an agent scope: unowned + own, never another agent's owned skill;
// the source-path map refreshes alongside.
const researcherContext = {
projectId: "project-1",
authToken: "token-1",
branchId: null,
agentId: "researcher",
availableSkillIds: ["stale-id"],
skillSourcePaths: { "stale-id": "skills/stale/SKILL.md" } as Readonly<
Record<string, string>
>,
};
await adapter.refreshProjectSkillIds(researcherContext);
assertEquals(researcherContext.availableSkillIds, ["builtin", "global", "researcher--cite"]);
assertEquals(researcherContext.skillSourcePaths, {
global: "skills/global/SKILL.md",
"researcher--cite": "agents/researcher/skills/cite/SKILL.md",
});

// Without an agent scope: conservative project-level rule (unowned only).
const projectContext = {
projectId: "project-1",
authToken: "token-1",
branchId: null,
availableSkillIds: [],
};
await adapter.refreshProjectSkillIds(projectContext);
assertEquals(projectContext.availableSkillIds, ["builtin", "global"]);
});
});
24 changes: 23 additions & 1 deletion src/agent/hosted/project-steering-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import type {
RuntimeSkillDefinition,
RuntimeSkillMetadataLogger,
} from "../runtime/skill-metadata.ts";
import { isRuntimeSkillVisibleTo } from "../runtime/skill-metadata.ts";

/** Public API contract for hosted project steering logger. */
export type HostedProjectSteeringLogger =
Expand All @@ -59,6 +60,13 @@ export type HostedProjectSteeringAdapterOptions = {
/** Context for hosted project skill IDs. */
export type HostedProjectSkillIdsContext = MutableAgentProjectContext & {
authToken: string;
/**
* Id of the agent this run executes as. Refreshes scope the rewritten
* skill set to this agent (unowned + own); when absent, the conservative
* project-level rule applies (unowned only) — a refresh can never widen
* visibility beyond the caller's scope.
*/
agentId?: string;
};

/** Public API contract for hosted project steering adapter. */
Expand Down Expand Up @@ -183,7 +191,21 @@ export function createHostedProjectSteeringAdapter(
branchId: context.branchId,
});

context.availableSkillIds = skills.map((skill) => skill.id);
// Owner-aware: the refreshed per-run skill set keeps the caller's
// scope — never another agent's owned skills — and the source-path
// map stays in sync so colocated skills do not go stale.
const visibleSkills = skills.filter((skill) =>
isRuntimeSkillVisibleTo(skill, { agentId: context.agentId })
);
context.availableSkillIds = visibleSkills.map((skill) => skill.id);
Comment thread
kojiwakayama marked this conversation as resolved.
const skillSourcePaths = Object.fromEntries(
visibleSkills
.filter((skill) => skill.sourcePath)
.map((skill) => [skill.id, skill.sourcePath as string]),
);
context.skillSourcePaths = Object.keys(skillSourcePaths).length > 0
? skillSourcePaths
: undefined;
},
};
}
12 changes: 4 additions & 8 deletions src/agent/hosted/veryfront-cloud-agent-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -742,21 +742,17 @@ function createAgentRuntime(
refreshSystem,
onSteeringMutation: async ({ mutation, taskContext }) => {
if (mutation.skillsChanged) {
await refreshProjectSkillIds(context, {
...taskContext,
authToken: taskContext.authToken,
});
// Pass the live task context (not a spread copy) so the refreshed
// owner-scoped skill ids and source paths actually land on the run.
await refreshProjectSkillIds(context, taskContext);
}
},
onStudioProjectSwitch: async ({ projectId, taskContext }) => {
if (!applyAgentProjectContextChange(taskContext, projectId)) {
return false;
}

await refreshProjectSkillIds(context, {
...taskContext,
authToken: taskContext.authToken,
});
await refreshProjectSkillIds(context, taskContext);
return true;
},
projectScopedRemoteToolOptions: {
Expand Down
2 changes: 2 additions & 0 deletions src/agent/project/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Deno.test("applyAgentProjectContextChange updates project and resets branch and
projectId: "project-1",
branchId: "branch-1",
availableSkillIds: ["skill-a"],
skillSourcePaths: { "skill-a": "skills/skill-a/SKILL.md" },
steeringRevision: 3,
};

Expand All @@ -21,6 +22,7 @@ Deno.test("applyAgentProjectContextChange updates project and resets branch and
projectId: "project-2",
branchId: null,
availableSkillIds: undefined,
skillSourcePaths: undefined,
steeringRevision: 3,
});
});
Expand Down
3 changes: 3 additions & 0 deletions src/agent/project/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ export interface MutableAgentProjectContext {
projectId: string;
branchId?: string | null;
availableSkillIds?: string[];
/** Per-run skill id -> discovered SKILL.md source path (owner-aware catalog). */
skillSourcePaths?: Readonly<Record<string, string>>;
}

/** Apply agent project context change helper. */
Expand All @@ -17,6 +19,7 @@ export function applyAgentProjectContextChange(
context.projectId = projectId;
context.branchId = null;
context.availableSkillIds = undefined;
context.skillSourcePaths = undefined;
return true;
}

Expand Down
59 changes: 59 additions & 0 deletions src/agent/runtime/project-skill-catalog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,62 @@ Deno.test("getRuntimeProjectSkillCatalog still parses legacy hidden project skil
assertEquals(skills.map((skill) => skill.id), ["legacy"]);
assertEquals(fileCalls.map((call) => call.path), [".veryfront/skills/legacy/SKILL.md"]);
});

// ── Colocated (agent-owned) skills in the catalog (review finding) ────────

const CITE_SKILL_MD = `---
name: cite
description: Cite sources properly
---
Cite primary sources.
`;

const RESEARCHER_SKILL_MD = `---
name: researcher
description: Research methodology
---
Follow the method.
`;

Deno.test("catalog includes colocated skills with owner metadata and source paths", async () => {
const { catalog } = createSkillCatalog({
paths: [
"agents/researcher/AGENT.md",
"agents/researcher/SKILL.md",
"agents/researcher/skills/cite/SKILL.md",
"agents/researcher/skills/cite/references/styles.md",
],
contentsByPath: {
"agents/researcher/SKILL.md": RESEARCHER_SKILL_MD,
"agents/researcher/skills/cite/SKILL.md": CITE_SKILL_MD,
},
});

const skills = await catalog();
const ids = skills.map((skill) => skill.id).sort();
assertEquals(ids, ["researcher", "researcher--cite"]);

const nested = skills.find((skill) => skill.id === "researcher--cite");
assertEquals(nested?.ownerAgentId, "researcher");
assertEquals(nested?.shortName, "cite");
assertEquals(nested?.sourcePath, "agents/researcher/skills/cite/SKILL.md");
assertEquals(nested?.references, ["references/styles.md"]);

const own = skills.find((skill) => skill.id === "researcher");
assertEquals(own?.ownerAgentId, "researcher");
assertEquals(own?.sourcePath, "agents/researcher/SKILL.md");
});

Deno.test("catalog keeps global skills unowned and carries their source paths", async () => {
const { catalog } = createSkillCatalog({
paths: ["skills/gmail/SKILL.md"],
contentsByPath: {
"skills/gmail/SKILL.md": CITE_SKILL_MD,
},
});

const skills = await catalog();
assertEquals(skills.length, 1);
assertEquals(skills[0]?.ownerAgentId, undefined);
assertEquals(skills[0]?.sourcePath, "skills/gmail/SKILL.md");
});
Loading