refactor(delegate-task): share buildSyncPromptTools between bootstrap and prompt dispatch
Two call sites built the sync delegate tool gate independently: sync-prompt-sender's prompt body construction and sync-task's bootstrap registration. Drift between them would let bootstrap claim one tool set while the actual prompt sent a different one. Extract buildSyncPromptTools and route both call sites through it so the registered bootstrap and the dispatched prompt always agree.
This commit is contained in:
@@ -1,18 +1,18 @@
|
|||||||
import type { DelegateTaskArgs, OpencodeClient, DelegatedModelConfig } from "./types"
|
|
||||||
import type { SisyphusAgentConfig } from "../../config/schema"
|
import type { SisyphusAgentConfig } from "../../config/schema"
|
||||||
import { isPlanFamily } from "./constants"
|
import { stripInvisibleAgentCharacters } from "../../shared/agent-display-names"
|
||||||
import { buildTaskPrompt } from "./prompt-builder"
|
import { getAgentToolRestrictions } from "../../shared/agent-tool-restrictions"
|
||||||
|
import { createInternalAgentTextPart } from "../../shared/internal-initiator-marker"
|
||||||
import {
|
import {
|
||||||
promptSyncWithModelSuggestionRetry,
|
promptSyncWithModelSuggestionRetry,
|
||||||
promptWithModelSuggestionRetry,
|
promptWithModelSuggestionRetry,
|
||||||
} from "../../shared/model-suggestion-retry"
|
} from "../../shared/model-suggestion-retry"
|
||||||
import { routePromptRetry, routePromptSyncRetry } from "../../shared/session-route"
|
|
||||||
import { formatDetailedError } from "./error-formatting"
|
|
||||||
import { getAgentToolRestrictions } from "../../shared/agent-tool-restrictions"
|
|
||||||
import { stripInvisibleAgentCharacters } from "../../shared/agent-display-names"
|
|
||||||
import { applySessionPromptParams } from "../../shared/session-prompt-params-helpers"
|
import { applySessionPromptParams } from "../../shared/session-prompt-params-helpers"
|
||||||
|
import { routePromptRetry, routePromptSyncRetry } from "../../shared/session-route"
|
||||||
import { setSessionTools } from "../../shared/session-tools-store"
|
import { setSessionTools } from "../../shared/session-tools-store"
|
||||||
import { createInternalAgentTextPart } from "../../shared/internal-initiator-marker"
|
import { isPlanFamily } from "./constants"
|
||||||
|
import { formatDetailedError } from "./error-formatting"
|
||||||
|
import { buildTaskPrompt } from "./prompt-builder"
|
||||||
|
import type { DelegatedModelConfig, DelegateTaskArgs, OpencodeClient } from "./types"
|
||||||
|
|
||||||
type SendSyncPromptDeps = {
|
type SendSyncPromptDeps = {
|
||||||
promptWithModelSuggestionRetry: typeof promptWithModelSuggestionRetry
|
promptWithModelSuggestionRetry: typeof promptWithModelSuggestionRetry
|
||||||
@@ -52,6 +52,15 @@ function isUnexpectedEofError(error: unknown): boolean {
|
|||||||
return lowered.includes("unexpected eof") || lowered.includes("json parse error")
|
return lowered.includes("unexpected eof") || lowered.includes("json parse error")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function buildSyncPromptTools(agentToUse: string): Record<string, boolean> {
|
||||||
|
return {
|
||||||
|
task: isPlanFamily(agentToUse),
|
||||||
|
call_omo_agent: true,
|
||||||
|
question: false,
|
||||||
|
...getAgentToolRestrictions(agentToUse),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export async function sendSyncPrompt(
|
export async function sendSyncPrompt(
|
||||||
client: OpencodeClient,
|
client: OpencodeClient,
|
||||||
input: {
|
input: {
|
||||||
@@ -67,15 +76,9 @@ export async function sendSyncPrompt(
|
|||||||
},
|
},
|
||||||
deps: SendSyncPromptDeps = sendSyncPromptDeps
|
deps: SendSyncPromptDeps = sendSyncPromptDeps
|
||||||
): Promise<string | null> {
|
): Promise<string | null> {
|
||||||
const allowTask = isPlanFamily(input.agentToUse)
|
|
||||||
const tddEnabled = input.sisyphusAgentConfig?.tdd
|
const tddEnabled = input.sisyphusAgentConfig?.tdd
|
||||||
const effectivePrompt = buildTaskPrompt(input.args.prompt, input.agentToUse, tddEnabled)
|
const effectivePrompt = buildTaskPrompt(input.args.prompt, input.agentToUse, tddEnabled)
|
||||||
const tools = {
|
const tools = buildSyncPromptTools(input.agentToUse)
|
||||||
task: allowTask,
|
|
||||||
call_omo_agent: true,
|
|
||||||
question: false,
|
|
||||||
...getAgentToolRestrictions(input.agentToUse),
|
|
||||||
}
|
|
||||||
setSessionTools(input.sessionID, tools)
|
setSessionTools(input.sessionID, tools)
|
||||||
|
|
||||||
applySessionPromptParams(input.sessionID, input.categoryModel)
|
applySessionPromptParams(input.sessionID, input.categoryModel)
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
const { describe, test, expect, beforeEach, afterEach, mock, spyOn } = require("bun:test")
|
import { describe, test, expect, beforeEach, afterEach, mock, spyOn } from "bun:test"
|
||||||
|
|
||||||
function clearRequireCache(modulePath: string): void {
|
function clearRequireCache(modulePath: string): void {
|
||||||
const resolvedPath = require.resolve(modulePath)
|
const resolvedPath = require.resolve(modulePath)
|
||||||
@@ -678,12 +678,16 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
|||||||
const { executeSyncTask } = require("./sync-task")
|
const { executeSyncTask } = require("./sync-task")
|
||||||
const { getDelegatedChildSessionBootstrap } = require("../../shared/delegated-child-session-bootstrap")
|
const { getDelegatedChildSessionBootstrap } = require("../../shared/delegated-child-session-bootstrap")
|
||||||
const observedBootstrapPrompts: string[] = []
|
const observedBootstrapPrompts: string[] = []
|
||||||
|
const observedBootstrapSystems: Array<string | undefined> = []
|
||||||
|
const observedBootstrapTools: Array<Record<string, boolean> | undefined> = []
|
||||||
|
|
||||||
const deps = {
|
const deps = {
|
||||||
createSyncSession: async () => ({ ok: true as const, sessionID: "ses_bootstrap_sync" }),
|
createSyncSession: async () => ({ ok: true as const, sessionID: "ses_bootstrap_sync" }),
|
||||||
sendSyncPrompt: async (_client: unknown, input: { sessionID: string }) => {
|
sendSyncPrompt: async (_client: unknown, input: { sessionID: string }) => {
|
||||||
const bootstrap = getDelegatedChildSessionBootstrap(input.sessionID)
|
const bootstrap = getDelegatedChildSessionBootstrap(input.sessionID)
|
||||||
observedBootstrapPrompts.push(bootstrap?.retryParts[0]?.text ?? "")
|
observedBootstrapPrompts.push(bootstrap?.retryParts[0]?.text ?? "")
|
||||||
|
observedBootstrapSystems.push(bootstrap?.system)
|
||||||
|
observedBootstrapTools.push(bootstrap?.tools)
|
||||||
return null
|
return null
|
||||||
},
|
},
|
||||||
pollSyncSession: async () => null,
|
pollSyncSession: async () => null,
|
||||||
@@ -717,10 +721,13 @@ describe("executeSyncTask - cleanup on error paths", () => {
|
|||||||
|
|
||||||
const result = await executeSyncTask(args, mockCtx, mockExecutorCtx, {
|
const result = await executeSyncTask(args, mockCtx, mockExecutorCtx, {
|
||||||
sessionID: "parent-session",
|
sessionID: "parent-session",
|
||||||
}, "sisyphus-junior", undefined, undefined, undefined, undefined, deps)
|
}, "sisyphus-junior", undefined, "sync delegated skill system", undefined, undefined, deps)
|
||||||
|
|
||||||
expect(result).toContain("sync result")
|
expect(result).toContain("sync result")
|
||||||
expect(observedBootstrapPrompts[0]).toContain("sync bootstrap prompt")
|
expect(observedBootstrapPrompts[0]).toContain("sync bootstrap prompt")
|
||||||
|
expect(observedBootstrapSystems[0]).toBe("sync delegated skill system")
|
||||||
|
expect(observedBootstrapTools[0]?.question).toBe(false)
|
||||||
|
expect(observedBootstrapTools[0]?.call_omo_agent).toBe(true)
|
||||||
expect(getDelegatedChildSessionBootstrap("ses_bootstrap_sync")).toBeUndefined()
|
expect(getDelegatedChildSessionBootstrap("ses_bootstrap_sync")).toBeUndefined()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import { formatDetailedError } from "./error-formatting"
|
|||||||
import type { ExecutorContext, ParentContext } from "./executor-types"
|
import type { ExecutorContext, ParentContext } from "./executor-types"
|
||||||
import { buildTaskPrompt } from "./prompt-builder"
|
import { buildTaskPrompt } from "./prompt-builder"
|
||||||
import { resolveMetadataModel } from "./resolve-metadata-model"
|
import { resolveMetadataModel } from "./resolve-metadata-model"
|
||||||
|
import { buildSyncPromptTools } from "./sync-prompt-sender"
|
||||||
import { type SyncTaskDeps, syncTaskDeps } from "./sync-task-deps"
|
import { type SyncTaskDeps, syncTaskDeps } from "./sync-task-deps"
|
||||||
import { getNextSyncFallbackModel, retrySyncPromptWithFallbacks } from "./sync-task-fallback"
|
import { getNextSyncFallbackModel, retrySyncPromptWithFallbacks } from "./sync-task-fallback"
|
||||||
import { formatDuration } from "./time-formatter"
|
import { formatDuration } from "./time-formatter"
|
||||||
@@ -117,6 +118,8 @@ export async function executeSyncTask(
|
|||||||
promptText: buildTaskPrompt(args.prompt, agentToUse, executorCtx.sisyphusAgentConfig?.tdd),
|
promptText: buildTaskPrompt(args.prompt, agentToUse, executorCtx.sisyphusAgentConfig?.tdd),
|
||||||
fallbackChain,
|
fallbackChain,
|
||||||
category: args.category,
|
category: args.category,
|
||||||
|
system: systemContent,
|
||||||
|
tools: buildSyncPromptTools(agentToUse),
|
||||||
modelFallbackControllerAccessor: executorCtx.modelFallbackControllerAccessor,
|
modelFallbackControllerAccessor: executorCtx.modelFallbackControllerAccessor,
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -138,7 +141,6 @@ export async function executeSyncTask(
|
|||||||
const publishSyncMetadata = async (
|
const publishSyncMetadata = async (
|
||||||
currentSessionID: string,
|
currentSessionID: string,
|
||||||
currentModel: DelegatedModelConfig | undefined,
|
currentModel: DelegatedModelConfig | undefined,
|
||||||
currentTaskId: string,
|
|
||||||
spawnDepth: number,
|
spawnDepth: number,
|
||||||
): Promise<void> => {
|
): Promise<void> => {
|
||||||
await publishToolMetadata(ctx, {
|
await publishToolMetadata(ctx, {
|
||||||
@@ -178,7 +180,7 @@ export async function executeSyncTask(
|
|||||||
modelInfo,
|
modelInfo,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
await publishSyncMetadata(sessionID, categoryModel, taskId, spawnContext.childDepth)
|
await publishSyncMetadata(sessionID, categoryModel, spawnContext.childDepth)
|
||||||
|
|
||||||
const syncPromptInput = {
|
const syncPromptInput = {
|
||||||
sessionID,
|
sessionID,
|
||||||
@@ -312,7 +314,7 @@ export async function executeSyncTask(
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
if (taskId) {
|
if (taskId) {
|
||||||
await publishSyncMetadata(activeSessionID, effectiveCategoryModel, taskId, spawnContext.childDepth)
|
await publishSyncMetadata(activeSessionID, effectiveCategoryModel, spawnContext.childDepth)
|
||||||
}
|
}
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -337,7 +339,7 @@ export async function executeSyncTask(
|
|||||||
modelRoutingNote = `\nModel: ${actualModelStr}${args.category ? ` (category: ${args.category})` : ""}`
|
modelRoutingNote = `\nModel: ${actualModelStr}${args.category ? ` (category: ${args.category})` : ""}`
|
||||||
}
|
}
|
||||||
|
|
||||||
await publishSyncMetadata(activeSessionID, effectiveCategoryModel, taskId!, spawnContext.childDepth)
|
await publishSyncMetadata(activeSessionID, effectiveCategoryModel, spawnContext.childDepth)
|
||||||
|
|
||||||
return `Task completed in ${duration}.
|
return `Task completed in ${duration}.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user