From 895cfe3bab2b4eae9ebc5e5f5287f764e776fa77 Mon Sep 17 00:00:00 2001 From: zhi Date: Thu, 2 Apr 2026 11:52:20 +0000 Subject: [PATCH] fix: align discussion workspace and tool schemas --- plugin/core/discussion-service.ts | 24 ++++++++++++----- plugin/core/discussion-state.ts | 1 + plugin/core/session-state.ts | 12 +++++++++ plugin/hooks/before-model-resolve.ts | 3 +++ plugin/index.ts | 4 +++ plugin/tools/register-tools.ts | 7 +++-- test/discussion-service.test.ts | 40 +++++++++++++++++++++++++++- test/register-tools.test.ts | 7 +++++ 8 files changed, 88 insertions(+), 10 deletions(-) diff --git a/plugin/core/discussion-service.ts b/plugin/core/discussion-service.ts index 1b4c536..8c89f7d 100644 --- a/plugin/core/discussion-service.ts +++ b/plugin/core/discussion-service.ts @@ -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 { 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([ + metadata.initiatorSessionId, + ...(deps.getDiscussionSessionKeys?.(metadata.discussionChannelId) || []), + ]); + for (const sessionKey of discussionSessionKeys) { + if (sessionKey) deps.forceNoReplyForSession(sessionKey); + } if (deps.moderatorBotToken) { const result = await sendModeratorMessage( diff --git a/plugin/core/discussion-state.ts b/plugin/core/discussion-state.ts index ebe79ac..0df1a91 100644 --- a/plugin/core/discussion-state.ts +++ b/plugin/core/discussion-state.ts @@ -6,6 +6,7 @@ export type DiscussionMetadata = { originChannelId: string; initiatorAgentId: string; initiatorSessionId: string; + initiatorWorkspaceRoot?: string; discussGuide: string; status: DiscussionStatus; createdAt: string; diff --git a/plugin/core/session-state.ts b/plugin/core/session-state.ts index 06187c8..1a3a028 100644 --- a/plugin/core/session-state.ts +++ b/plugin/core/session-state.ts @@ -16,6 +16,18 @@ export const sessionChannelId = new Map(); export const sessionAccountId = new Map(); export const sessionTurnHandled = new Set(); export const forceNoReplySessions = new Set(); +export const discussionChannelSessions = new Map>(); + +export function recordDiscussionSession(channelId: string, sessionKey: string): void { + if (!channelId || !sessionKey) return; + const current = discussionChannelSessions.get(channelId) || new Set(); + current.add(sessionKey); + discussionChannelSessions.set(channelId, current); +} + +export function getDiscussionSessionKeys(channelId: string): string[] { + return [...(discussionChannelSessions.get(channelId) || new Set())]; +} export function pruneDecisionMap(now = Date.now()): void { for (const [k, v] of sessionDecision.entries()) { diff --git a/plugin/hooks/before-model-resolve.ts b/plugin/hooks/before-model-resolve.ts index 48b8283..cda943a 100644 --- a/plugin/hooks/before-model-resolve.ts +++ b/plugin/hooks/before-model-resolve.ts @@ -21,6 +21,7 @@ type BeforeModelResolveDeps = { sessionAllowed: Map; sessionChannelId: Map; sessionAccountId: Map; + recordDiscussionSession?: (channelId: string, sessionKey: string) => void; forceNoReplySessions: Set; policyState: { channelPolicies: Record }; 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}`); diff --git a/plugin/index.ts b/plugin/index.ts index 1d95b98..03bac0b 100644 --- a/plugin/index.ts +++ b/plugin/index.ts @@ -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, diff --git a/plugin/tools/register-tools.ts b/plugin/tools/register-tools.ts index b941c80..4730bde 100644 --- a/plugin/tools/register-tools.ts +++ b/plugin/tools/register-tools.ts @@ -13,6 +13,7 @@ type ToolDeps = { originChannelId: string; initiatorAgentId: string; initiatorSessionId: string; + initiatorWorkspaceRoot?: string; discussGuide: string; }) => Promise; 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), __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: { diff --git a/test/discussion-service.test.ts b/test/discussion-service.test.ts index d4198dc..fb70a2c 100644 --- a/test/discussion-service.test.ts +++ b/test/discussion-service.test.ts @@ -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.', }); diff --git a/test/register-tools.test.ts b/test/register-tools.test.ts index 608dfa2..516317f 100644 --- a/test/register-tools.test.ts +++ b/test/register-tools.test.ts @@ -5,6 +5,7 @@ import { registerDirigentTools } from '../plugin/tools/register-tools.ts'; type RegisteredTool = { name: string; + parameters?: Record; handler: (params: Record, ctx?: Record) => Promise; }; @@ -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',