forked from EduCraft/curriculum-project-hub
fix(hub): expose TodoWrite via SDK default toolset
Unrestricted roles were still passed an explicit --tools name list. The native Claude binary only reliably registers bundled tools like TodoWrite on --tools default. Treat role tools JSON null as unrestricted, use the claude_code preset (→ default) in that case, and keep TodoWrite on allowedTools.
This commit is contained in:
@@ -77,8 +77,11 @@ const SUPPORTED_ROLE_TOOLS = new Set([
|
|||||||
...Object.keys(ROLE_TOOL_TO_CPH_HUB_MCP_TOOLS),
|
...Object.keys(ROLE_TOOL_TO_CPH_HUB_MCP_TOOLS),
|
||||||
]);
|
]);
|
||||||
|
|
||||||
export function claudeSdkToolConfigForRole(roleTools: readonly string[] | undefined): ClaudeSdkToolConfig {
|
export function claudeSdkToolConfigForRole(
|
||||||
if (roleTools === undefined) {
|
roleTools: readonly string[] | null | undefined,
|
||||||
|
): ClaudeSdkToolConfig {
|
||||||
|
// DB/runtime "unrestricted" is JSON null; treat the same as undefined.
|
||||||
|
if (roleTools === undefined || roleTools === null) {
|
||||||
const mcpTools = CPH_HUB_MCP_TOOL_IDS.map(claudeMcpToolName);
|
const mcpTools = CPH_HUB_MCP_TOOL_IDS.map(claudeMcpToolName);
|
||||||
return {
|
return {
|
||||||
tools: [...DEFAULT_CLAUDE_BUILT_IN_TOOLS],
|
tools: [...DEFAULT_CLAUDE_BUILT_IN_TOOLS],
|
||||||
@@ -103,8 +106,10 @@ export function claudeSdkToolConfigForRole(roleTools: readonly string[] | undefi
|
|||||||
return { tools: builtIns, allowedTools };
|
return { tools: builtIns, allowedTools };
|
||||||
}
|
}
|
||||||
|
|
||||||
export function cphHubMcpToolsForRole(roleTools: readonly string[] | undefined): readonly CphHubMcpToolId[] {
|
export function cphHubMcpToolsForRole(
|
||||||
if (roleTools === undefined) return [...CPH_HUB_MCP_TOOL_IDS];
|
roleTools: readonly string[] | null | undefined,
|
||||||
|
): readonly CphHubMcpToolId[] {
|
||||||
|
if (roleTools === undefined || roleTools === null) return [...CPH_HUB_MCP_TOOL_IDS];
|
||||||
|
|
||||||
const tools: CphHubMcpToolId[] = [];
|
const tools: CphHubMcpToolId[] = [];
|
||||||
for (const roleTool of roleTools) {
|
for (const roleTool of roleTools) {
|
||||||
@@ -116,8 +121,11 @@ export function cphHubMcpToolsForRole(roleTools: readonly string[] | undefined):
|
|||||||
return tools;
|
return tools;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function roleToolsAllow(roleTools: readonly string[] | undefined, roleTool: string): boolean {
|
export function roleToolsAllow(
|
||||||
if (roleTools === undefined) return true;
|
roleTools: readonly string[] | null | undefined,
|
||||||
|
roleTool: string,
|
||||||
|
): boolean {
|
||||||
|
if (roleTools === undefined || roleTools === null) return true;
|
||||||
for (const configured of roleTools) {
|
for (const configured of roleTools) {
|
||||||
assertSupportedRoleTool(configured);
|
assertSupportedRoleTool(configured);
|
||||||
if (configured === roleTool) return true;
|
if (configured === roleTool) return true;
|
||||||
|
|||||||
+20
-12
@@ -140,7 +140,10 @@ export async function runAgent(req: RunRequest): Promise<RunResult> {
|
|||||||
let cleanupSecurity = async (): Promise<void> => {};
|
let cleanupSecurity = async (): Promise<void> => {};
|
||||||
try {
|
try {
|
||||||
await persistAgentMessage(req, "user", req.prompt);
|
await persistAgentMessage(req, "user", req.prompt);
|
||||||
const toolConfig = claudeSdkToolConfigForRole(req.tools);
|
// Role tools JSON null means unrestricted (omit), not "deny all".
|
||||||
|
const roleToolIds = req.tools === null ? undefined : req.tools;
|
||||||
|
const unrestricted = roleToolIds === undefined;
|
||||||
|
const toolConfig = claudeSdkToolConfigForRole(roleToolIds);
|
||||||
const workspaceRoot = req.project.workspaceRoot?.trim();
|
const workspaceRoot = req.project.workspaceRoot?.trim();
|
||||||
if (workspaceRoot === undefined || workspaceRoot === "") {
|
if (workspaceRoot === undefined || workspaceRoot === "") {
|
||||||
throw new Error("Agent run requires the configured workspace root");
|
throw new Error("Agent run requires the configured workspace root");
|
||||||
@@ -154,20 +157,25 @@ export async function runAgent(req: RunRequest): Promise<RunResult> {
|
|||||||
});
|
});
|
||||||
cleanupSecurity = security.cleanup;
|
cleanupSecurity = security.cleanup;
|
||||||
const hasSkills = security.skillIds.length > 0;
|
const hasSkills = security.skillIds.length > 0;
|
||||||
|
|
||||||
type QueryOptions = NonNullable<Parameters<typeof query>[0]["options"]>;
|
type QueryOptions = NonNullable<Parameters<typeof query>[0]["options"]>;
|
||||||
|
// When unrestricted, pass the SDK default toolset (`--tools default`) instead of
|
||||||
|
// an explicit subset. Native claude uses that path to register bundled tools like
|
||||||
|
// TodoWrite; listing names alone can omit them from the model's function list.
|
||||||
|
// allowedTools still carries explicit MCP names + TodoWrite for permission.
|
||||||
|
const skillExtras = hasSkills ? (["Skill"] as const) : ([] as const);
|
||||||
|
const toolsOption: QueryOptions["tools"] = unrestricted
|
||||||
|
? { type: "preset", preset: "claude_code" }
|
||||||
|
: uniqueTools([...toolConfig.tools, "TodoWrite", ...skillExtras]);
|
||||||
|
const allowedToolsOption = uniqueTools([
|
||||||
|
...toolConfig.allowedTools,
|
||||||
|
"TodoWrite",
|
||||||
|
...skillExtras,
|
||||||
|
]);
|
||||||
|
|
||||||
const options: QueryOptions = {
|
const options: QueryOptions = {
|
||||||
cwd: security.cwd,
|
cwd: security.cwd,
|
||||||
// `skills` controls discovery/allowlisting, but an explicit `tools`
|
tools: toolsOption,
|
||||||
// list still has to expose the Skill dispatcher itself.
|
allowedTools: allowedToolsOption,
|
||||||
// TodoWrite is always available so multi-step runs can surface a live checklist
|
|
||||||
// on the Feishu card (Manus-style progress), even when a role whitelists tools.
|
|
||||||
tools: uniqueTools([
|
|
||||||
...toolConfig.tools,
|
|
||||||
"TodoWrite",
|
|
||||||
...(hasSkills ? ["Skill"] : []),
|
|
||||||
]),
|
|
||||||
allowedTools: uniqueTools([...toolConfig.allowedTools, "TodoWrite"]),
|
|
||||||
maxTurns: cap,
|
maxTurns: cap,
|
||||||
includePartialMessages: true,
|
includePartialMessages: true,
|
||||||
// ADR-0018: bypass interactive prompts (headless server); the sandbox
|
// ADR-0018: bypass interactive prompts (headless server); the sandbox
|
||||||
|
|||||||
@@ -115,7 +115,7 @@ describe("runAgent", () => {
|
|||||||
settingSources: [],
|
settingSources: [],
|
||||||
settings: { disableBundledSkills: true, todoFeatureEnabled: true },
|
settings: { disableBundledSkills: true, todoFeatureEnabled: true },
|
||||||
skills: [],
|
skills: [],
|
||||||
tools: expect.arrayContaining(["TodoWrite"]),
|
tools: { type: "preset", preset: "claude_code" },
|
||||||
allowedTools: expect.arrayContaining(["TodoWrite"]),
|
allowedTools: expect.arrayContaining(["TodoWrite"]),
|
||||||
strictMcpConfig: true,
|
strictMcpConfig: true,
|
||||||
sandbox: expect.objectContaining({
|
sandbox: expect.objectContaining({
|
||||||
|
|||||||
Reference in New Issue
Block a user