fix: align discussion workspace and tool schemas

This commit is contained in:
zhi
2026-04-02 11:52:20 +00:00
parent 7bccb660df
commit 895cfe3bab
8 changed files with 88 additions and 10 deletions

View File

@@ -16,16 +16,18 @@ type DiscussionServiceDeps = {
moderatorUserId?: string;
workspaceRoot?: string;
forceNoReplyForSession: (sessionKey: string) => void;
getDiscussionSessionKeys?: (channelId: string) => string[];
};
export function createDiscussionService(deps: DiscussionServiceDeps) {
const workspaceRoot = path.resolve(deps.workspaceRoot || process.cwd());
const defaultWorkspaceRoot = path.resolve(deps.workspaceRoot || process.cwd());
async function initDiscussion(params: {
discussionChannelId: string;
originChannelId: string;
initiatorAgentId: string;
initiatorSessionId: string;
initiatorWorkspaceRoot?: string;
discussGuide: string;
}): Promise<DiscussionMetadata> {
const metadata = createDiscussion({
@@ -34,6 +36,7 @@ export function createDiscussionService(deps: DiscussionServiceDeps) {
originChannelId: params.originChannelId,
initiatorAgentId: params.initiatorAgentId,
initiatorSessionId: params.initiatorSessionId,
initiatorWorkspaceRoot: params.initiatorWorkspaceRoot,
discussGuide: params.discussGuide,
status: "active",
createdAt: new Date().toISOString(),
@@ -71,17 +74,18 @@ export function createDiscussionService(deps: DiscussionServiceDeps) {
}
}
function validateSummaryPath(summaryPath: string): string {
function validateSummaryPath(summaryPath: string, workspaceRoot?: string): string {
if (!summaryPath || !summaryPath.trim()) throw new Error("summaryPath is required");
const resolved = path.resolve(workspaceRoot, summaryPath);
const relative = path.relative(workspaceRoot, resolved);
const effectiveWorkspaceRoot = path.resolve(workspaceRoot || defaultWorkspaceRoot);
const resolved = path.resolve(effectiveWorkspaceRoot, summaryPath);
const relative = path.relative(effectiveWorkspaceRoot, resolved);
if (relative.startsWith("..") || path.isAbsolute(relative)) {
throw new Error("summaryPath must stay inside the initiator workspace");
}
const real = fs.realpathSync.native(resolved);
const realWorkspace = fs.realpathSync.native(workspaceRoot);
const realWorkspace = fs.realpathSync.native(effectiveWorkspaceRoot);
const realRelative = path.relative(realWorkspace, real);
if (realRelative.startsWith("..") || path.isAbsolute(realRelative)) {
throw new Error("summaryPath resolves outside the initiator workspace");
@@ -108,11 +112,17 @@ export function createDiscussionService(deps: DiscussionServiceDeps) {
throw new Error("only the discussion initiator agent may call discuss-callback");
}
const realPath = validateSummaryPath(params.summaryPath);
const realPath = validateSummaryPath(params.summaryPath, metadata.initiatorWorkspaceRoot);
const closed = closeDiscussion(params.channelId, realPath);
if (!closed) throw new Error("failed to close discussion");
deps.forceNoReplyForSession(metadata.initiatorSessionId);
const discussionSessionKeys = new Set<string>([
metadata.initiatorSessionId,
...(deps.getDiscussionSessionKeys?.(metadata.discussionChannelId) || []),
]);
for (const sessionKey of discussionSessionKeys) {
if (sessionKey) deps.forceNoReplyForSession(sessionKey);
}
if (deps.moderatorBotToken) {
const result = await sendModeratorMessage(

View File

@@ -6,6 +6,7 @@ export type DiscussionMetadata = {
originChannelId: string;
initiatorAgentId: string;
initiatorSessionId: string;
initiatorWorkspaceRoot?: string;
discussGuide: string;
status: DiscussionStatus;
createdAt: string;

View File

@@ -16,6 +16,18 @@ export const sessionChannelId = new Map<string, string>();
export const sessionAccountId = new Map<string, string>();
export const sessionTurnHandled = new Set<string>();
export const forceNoReplySessions = new Set<string>();
export const discussionChannelSessions = new Map<string, Set<string>>();
export function recordDiscussionSession(channelId: string, sessionKey: string): void {
if (!channelId || !sessionKey) return;
const current = discussionChannelSessions.get(channelId) || new Set<string>();
current.add(sessionKey);
discussionChannelSessions.set(channelId, current);
}
export function getDiscussionSessionKeys(channelId: string): string[] {
return [...(discussionChannelSessions.get(channelId) || new Set<string>())];
}
export function pruneDecisionMap(now = Date.now()): void {
for (const [k, v] of sessionDecision.entries()) {

View File

@@ -21,6 +21,7 @@ type BeforeModelResolveDeps = {
sessionAllowed: Map<string, boolean>;
sessionChannelId: Map<string, string>;
sessionAccountId: Map<string, string>;
recordDiscussionSession?: (channelId: string, sessionKey: string) => void;
forceNoReplySessions: Set<string>;
policyState: { channelPolicies: Record<string, unknown> };
DECISION_TTL_MS: number;
@@ -43,6 +44,7 @@ export function registerBeforeModelResolveHook(deps: BeforeModelResolveDeps): vo
sessionAllowed,
sessionChannelId,
sessionAccountId,
recordDiscussionSession,
forceNoReplySessions,
policyState,
DECISION_TTL_MS,
@@ -92,6 +94,7 @@ export function registerBeforeModelResolveHook(deps: BeforeModelResolveDeps): vo
if (derived.channelId) {
sessionChannelId.set(key, derived.channelId);
recordDiscussionSession?.(derived.channelId, key);
if (discussionService?.isClosedDiscussion(derived.channelId)) {
sessionAllowed.set(key, false);
api.logger.info(`dirigent: before_model_resolve forcing no-reply for closed discussion channel=${derived.channelId} session=${key}`);

View File

@@ -22,7 +22,9 @@ import { enterMultiMessageMode, exitMultiMessageMode, isMultiMessageMode } from
import {
DECISION_TTL_MS,
forceNoReplySessions,
getDiscussionSessionKeys,
pruneDecisionMap,
recordDiscussionSession,
sessionAccountId,
sessionAllowed,
sessionChannelId,
@@ -127,6 +129,7 @@ export default {
forceNoReplyForSession: (sessionKey: string) => {
if (sessionKey) forceNoReplySessions.add(sessionKey);
},
getDiscussionSessionKeys,
});
// Register tools
@@ -162,6 +165,7 @@ export default {
sessionAllowed,
sessionChannelId,
sessionAccountId,
recordDiscussionSession,
forceNoReplySessions,
policyState,
DECISION_TTL_MS,

View File

@@ -13,6 +13,7 @@ type ToolDeps = {
originChannelId: string;
initiatorAgentId: string;
initiatorSessionId: string;
initiatorWorkspaceRoot?: string;
discussGuide: string;
}) => Promise<unknown>;
handleCallback: (params: {
@@ -119,6 +120,7 @@ export function registerDirigentTools(deps: ToolDeps): void {
originChannelId: callbackChannelId,
initiatorAgentId: String((params.__agentId as string | undefined) || ""),
initiatorSessionId: String((params.__sessionKey as string | undefined) || ""),
initiatorWorkspaceRoot: typeof params.__workspaceRoot === "string" ? params.__workspaceRoot : undefined,
discussGuide,
});
}
@@ -161,7 +163,7 @@ export function registerDirigentTools(deps: ToolDeps): void {
api.registerTool({
name: "dirigent_discord_control",
description: "Create/update Discord private channels using the configured Discord bot token",
inputSchema: {
parameters: {
type: "object",
additionalProperties: false,
properties: {
@@ -194,6 +196,7 @@ export function registerDirigentTools(deps: ToolDeps): void {
...(params as Record<string, unknown>),
__agentId: ctx?.agentId,
__sessionKey: ctx?.sessionKey,
__workspaceRoot: ctx?.workspaceRoot,
};
return executeDiscordAction(params.action as DiscordControlAction, nextParams);
},
@@ -202,7 +205,7 @@ export function registerDirigentTools(deps: ToolDeps): void {
api.registerTool({
name: "discuss-callback",
description: "Close a discussion channel and notify the origin work channel with the discussion summary path",
inputSchema: {
parameters: {
type: "object",
additionalProperties: false,
properties: {

View File

@@ -62,6 +62,7 @@ test('handleCallback closes an active discussion and records the resolved summar
api: makeApi() as any,
workspaceRoot: workspace,
forceNoReplyForSession: (sessionKey) => forcedSessions.push(sessionKey),
getDiscussionSessionKeys: () => ['session-beta-helper'],
});
await service.initDiscussion({
@@ -69,6 +70,7 @@ test('handleCallback closes an active discussion and records the resolved summar
originChannelId: 'origin-2',
initiatorAgentId: 'agent-beta',
initiatorSessionId: 'session-beta',
initiatorWorkspaceRoot: workspace,
discussGuide: 'Write the wrap-up.',
});
@@ -84,7 +86,7 @@ test('handleCallback closes an active discussion and records the resolved summar
assert.equal(result.discussion.status, 'closed');
assert.equal(result.discussion.summaryPath, fs.realpathSync.native(summaryAbsPath));
assert.ok(result.discussion.completedAt);
assert.deepEqual(forcedSessions, ['session-beta']);
assert.deepEqual(forcedSessions.sort(), ['session-beta', 'session-beta-helper']);
});
test('handleCallback rejects duplicate callback after the discussion is already closed', async () => {
@@ -142,6 +144,7 @@ test('handleCallback accepts a valid summaryPath inside the initiator workspace'
originChannelId: 'origin-4',
initiatorAgentId: 'agent-delta',
initiatorSessionId: 'session-delta',
initiatorWorkspaceRoot: workspace,
discussGuide: 'Path validation.',
});
@@ -181,6 +184,39 @@ test('handleCallback rejects a missing summary file', async () => {
);
});
test('handleCallback uses the initiator workspace root instead of the process cwd', async () => {
const workspace = makeWorkspace();
const summaryRelPath = path.join('notes', 'initiator-only.md');
const summaryAbsPath = path.join(workspace, summaryRelPath);
fs.mkdirSync(path.dirname(summaryAbsPath), { recursive: true });
fs.writeFileSync(summaryAbsPath, 'initiator workspace file\n');
const differentDefaultWorkspace = makeWorkspace();
const service = createDiscussionService({
api: makeApi() as any,
workspaceRoot: differentDefaultWorkspace,
forceNoReplyForSession: () => {},
});
await service.initDiscussion({
discussionChannelId: 'discussion-workspace-root-1',
originChannelId: 'origin-6a',
initiatorAgentId: 'agent-zeta-root',
initiatorSessionId: 'session-zeta-root',
initiatorWorkspaceRoot: workspace,
discussGuide: 'Use initiator workspace root.',
});
const result = await service.handleCallback({
channelId: 'discussion-workspace-root-1',
summaryPath: summaryRelPath,
callerAgentId: 'agent-zeta-root',
callerSessionKey: 'session-zeta-root',
});
assert.equal(result.summaryPath, fs.realpathSync.native(summaryAbsPath));
});
test('handleCallback rejects .. path traversal outside the initiator workspace', async () => {
const workspace = makeWorkspace();
const outsideDir = makeWorkspace();
@@ -198,6 +234,7 @@ test('handleCallback rejects .. path traversal outside the initiator workspace',
originChannelId: 'origin-6',
initiatorAgentId: 'agent-zeta',
initiatorSessionId: 'session-zeta',
initiatorWorkspaceRoot: workspace,
discussGuide: 'Reject traversal.',
});
@@ -232,6 +269,7 @@ test('handleCallback rejects an absolute path outside the initiator workspace',
originChannelId: 'origin-7',
initiatorAgentId: 'agent-eta',
initiatorSessionId: 'session-eta',
initiatorWorkspaceRoot: workspace,
discussGuide: 'Reject absolute outside path.',
});

View File

@@ -5,6 +5,7 @@ import { registerDirigentTools } from '../plugin/tools/register-tools.ts';
type RegisteredTool = {
name: string;
parameters?: Record<string, unknown>;
handler: (params: Record<string, unknown>, ctx?: Record<string, unknown>) => Promise<any>;
};
@@ -62,6 +63,7 @@ test('plain private channel create works unchanged without discussion params', a
const tool = api.tools.get('dirigent_discord_control');
assert.ok(tool);
assert.ok(tool!.parameters);
const result = await tool!.handler({
action: 'channel-private-create',
@@ -100,6 +102,7 @@ test('private channel create rejects callbackChannelId without discussGuide', as
const tool = api.tools.get('dirigent_discord_control');
assert.ok(tool);
assert.ok(tool!.parameters);
const result = await tool!.handler({
action: 'channel-private-create',
@@ -142,6 +145,7 @@ test('discussion-mode channel create initializes discussion metadata', async ()
const tool = api.tools.get('dirigent_discord_control');
assert.ok(tool);
assert.ok(tool!.parameters);
const result = await tool!.handler({
action: 'channel-private-create',
@@ -152,6 +156,7 @@ test('discussion-mode channel create initializes discussion metadata', async ()
}, {
agentId: 'agent-a',
sessionKey: 'session-a',
workspaceRoot: '/workspace/agent-a',
});
assert.equal(result.isError, undefined);
@@ -162,6 +167,7 @@ test('discussion-mode channel create initializes discussion metadata', async ()
originChannelId: 'origin-1',
initiatorAgentId: 'agent-a',
initiatorSessionId: 'session-a',
initiatorWorkspaceRoot: '/workspace/agent-a',
discussGuide: 'Decide the callback contract.',
});
} finally {
@@ -190,6 +196,7 @@ test('discuss-callback registers and forwards channel/session/agent context', as
const tool = api.tools.get('discuss-callback');
assert.ok(tool);
assert.ok(tool!.parameters);
const result = await tool!.handler({ summaryPath: 'plans/summary.md' }, {
channelId: 'discussion-1',