diff --git a/gitnexus/src/core/move/README.md b/gitnexus/src/core/move/README.md index 3a6f3fb86..6d9bb9c3e 100644 --- a/gitnexus/src/core/move/README.md +++ b/gitnexus/src/core/move/README.md @@ -6,6 +6,10 @@ MCP, and projects compiler facts into the standard GitNexus knowledge graph. Declaration and semantic data come from the compiler-backed `facts` and `call_graph` queries rather than raw-source parsing. +Cold compiler builds for large packages may take several minutes. Tool calls +default to a five-minute timeout; override it in milliseconds with +`GITNEXUS_MOVE_FLOW_TIMEOUT_MS` when a repository needs a larger budget. + ```text Move package -> move-flow MCP diff --git a/gitnexus/src/core/move/constants.ts b/gitnexus/src/core/move/constants.ts index e371e7824..1801d0943 100644 --- a/gitnexus/src/core/move/constants.ts +++ b/gitnexus/src/core/move/constants.ts @@ -5,6 +5,8 @@ * with no compile error, so they live here. */ +import path from 'node:path'; + /** `NodeProperties.language` tag for every Move symbol. */ export const MOVE_LANGUAGE = 'move'; @@ -104,9 +106,10 @@ export function moveAvailabilityRequiresFullRebuild( */ export function moveRepoRelativePath(absPath: string, repoPath?: string): string { if (!repoPath) return absPath; - if (absPath.startsWith(repoPath)) { - const rel = absPath.slice(repoPath.length); - return rel.startsWith('/') ? rel.slice(1) : rel; + const relative = path.relative(repoPath, absPath); + if (relative === '') return relative; + if (relative === '..' || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) { + return absPath; } - return absPath; + return relative.replaceAll(path.sep, '/'); } diff --git a/gitnexus/src/core/move/mcp-client.ts b/gitnexus/src/core/move/mcp-client.ts index d1bc53a47..7c05ba9b4 100644 --- a/gitnexus/src/core/move/mcp-client.ts +++ b/gitnexus/src/core/move/mcp-client.ts @@ -47,6 +47,16 @@ function isMcpListToolResult(v: unknown): v is McpListToolResult { * (and some builds route package build failures through it). */ const JSON_RPC_INVALID_PARAMS = -32602; +/** Real Aptos packages may need several minutes for a cold compiler build. */ +const DEFAULT_MOVE_FLOW_TOOL_TIMEOUT_MS = 300_000; + +function resolveMoveFlowToolTimeoutMs(): number { + const configured = Number(process.env.GITNEXUS_MOVE_FLOW_TIMEOUT_MS); + return Number.isSafeInteger(configured) && configured > 0 + ? configured + : DEFAULT_MOVE_FLOW_TOOL_TIMEOUT_MS; +} + /** * A move-flow tool call that failed in user space: the package could not be * built (an `isError: true` tool result whose content text carries the @@ -390,6 +400,7 @@ export class MoveFlowMcpClient implements MoveFlowClient { return new Promise((resolve, reject) => { const id = ++this.requestId; + const timeoutMs = resolveMoveFlowToolTimeoutMs(); const timeout = setTimeout(() => { this.pending.delete(id); try { @@ -397,8 +408,13 @@ export class MoveFlowMcpClient implements MoveFlowClient { } catch { /* process may already be dead */ } - reject(new Error(`move-flow '${toolName}' timed out after 120s`)); - }, 120000); + reject( + new Error( + `move-flow '${toolName}' timed out after ${timeoutMs}ms ` + + '(raise GITNEXUS_MOVE_FLOW_TIMEOUT_MS for large packages)', + ), + ); + }, timeoutMs); this.pending.set(id, { resolve: (result) => { diff --git a/gitnexus/test/unit/move/constants.test.ts b/gitnexus/test/unit/move/constants.test.ts new file mode 100644 index 000000000..8ec837fc6 --- /dev/null +++ b/gitnexus/test/unit/move/constants.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from 'vitest'; +import path from 'node:path'; +import { moveRepoRelativePath } from '../../../src/core/move/constants.js'; + +describe('moveRepoRelativePath', () => { + it('returns a slash-normalized repo-relative path on the host platform', () => { + const repoRoot = path.resolve('/repo'); + const source = path.join(repoRoot, 'pkg', 'sources', 'module.move'); + + expect(moveRepoRelativePath(source, repoRoot)).toBe('pkg/sources/module.move'); + }); + + it('does not claim a sibling path that merely shares the repo prefix', () => { + const repoRoot = path.resolve('/repo/pkg'); + const sibling = path.resolve('/repo/pkg_other/sources/module.move'); + + expect(moveRepoRelativePath(sibling, repoRoot)).toBe(sibling); + }); +}); diff --git a/gitnexus/test/unit/move/mcp-client.test.ts b/gitnexus/test/unit/move/mcp-client.test.ts index 55bc65b16..96710b9e8 100644 --- a/gitnexus/test/unit/move/mcp-client.test.ts +++ b/gitnexus/test/unit/move/mcp-client.test.ts @@ -79,10 +79,12 @@ describe('tryCreateMoveFlowClient', () => { describe('MoveFlowMcpClient', () => { beforeEach(() => { mockSpawn.mockReset(); + delete process.env.GITNEXUS_MOVE_FLOW_TIMEOUT_MS; }); afterEach(() => { vi.useRealTimers(); + delete process.env.GITNEXUS_MOVE_FLOW_TIMEOUT_MS; }); it('shutdown clears all state', async () => { @@ -159,6 +161,34 @@ describe('MoveFlowMcpClient', () => { await client.shutdown(); }); + it('honours the Move tool timeout override for large or test packages', async () => { + vi.useFakeTimers(); + process.env.GITNEXUS_MOVE_FLOW_TIMEOUT_MS = '25'; + const proc = createMockProc(); + mockSpawn.mockReturnValue(proc as any); + + proc.stdin.on('data', (chunk: Buffer) => { + for (const line of chunk.toString().split('\n')) { + if (!line.trim()) continue; + const msg = JSON.parse(line); + if (msg.method === 'initialize') { + proc.stdout.write(JSON.stringify({ jsonrpc: '2.0', id: msg.id, result: {} }) + '\n'); + } + } + }); + + const client = new MoveFlowMcpClient('move-flow'); + const request = client.facts('/slow'); + const failure = expect(request).rejects.toThrow( + "move-flow 'move_package_query' timed out after 25ms", + ); + + await vi.advanceTimersByTimeAsync(25); + await failure; + expect(proc.kill).toHaveBeenCalledOnce(); + await client.shutdown(); + }); + it('clears initialization state when move-flow exits during launch so the next call retries', async () => { const crashedProc = createMockProc(); const retryProc = createMockProc(); diff --git a/gitnexus/test/unit/move/move-ingest-empty-facts.test.ts b/gitnexus/test/unit/move/move-ingest-empty-facts.test.ts index f02bd1ffc..f1fd0e760 100644 --- a/gitnexus/test/unit/move/move-ingest-empty-facts.test.ts +++ b/gitnexus/test/unit/move/move-ingest-empty-facts.test.ts @@ -8,9 +8,12 @@ * (with the compiler diagnostics), status unavailable -> error (old behavior). */ import { describe, it, expect } from 'vitest'; +import path from 'node:path'; import type { MoveFlowClient } from '../../../src/core/move/mcp-client.js'; import { runMoveIngestPhase } from '../../helpers/move-ingest-harness.js'; +const REPO_ROOT = path.resolve('/repo'); + function makeClient(overrides: Partial = {}): MoveFlowClient { return { facts: async () => ({}), @@ -24,7 +27,7 @@ function makeClient(overrides: Partial = {}): MoveFlowClient { /** Run the phase against a fake repo with one package holding one .move file. */ async function runPhase(client: MoveFlowClient) { - return runMoveIngestPhase(client, '/repo', ['pkg/Move.toml', 'pkg/sources/t.move']); + return runMoveIngestPhase(client, REPO_ROOT, ['pkg/Move.toml', 'pkg/sources/t.move']); } function emptyFactsIssues(output: Awaited>) { @@ -93,7 +96,7 @@ describe('moveIngest empty-facts discrimination', () => { makeClient({ facts: async () => ({ '0xa::t': { - file: '/repo/pkg/sources/t.move', + file: path.join(REPO_ROOT, 'pkg', 'sources', 't.move'), span: [1, 3] as [number, number], friends: [], attributes: [], diff --git a/gitnexus/test/unit/move/move-ingest-ownership.test.ts b/gitnexus/test/unit/move/move-ingest-ownership.test.ts index b4d3088ab..23e5a832e 100644 --- a/gitnexus/test/unit/move/move-ingest-ownership.test.ts +++ b/gitnexus/test/unit/move/move-ingest-ownership.test.ts @@ -5,9 +5,12 @@ * bare startsWith('/repo/pkg_a') would claim '/repo/pkg_ab/sources/x.move'. */ import { describe, it, expect } from 'vitest'; +import path from 'node:path'; import type { MoveFlowClient } from '../../../src/core/move/mcp-client.js'; import { runMoveIngestPhase } from '../../helpers/move-ingest-harness.js'; +const REPO_ROOT = path.resolve('/repo'); + /** Client returning empty facts for every package, without a status tool. */ function emptyFactsClient(): MoveFlowClient { return { @@ -21,7 +24,7 @@ function emptyFactsClient(): MoveFlowClient { describe('moveIngest package-ownership attribution', () => { it('attributes each file to its own package across sibling packages pkg_a / pkg_ab', async () => { - const output = await runMoveIngestPhase(emptyFactsClient(), '/repo', [ + const output = await runMoveIngestPhase(emptyFactsClient(), REPO_ROOT, [ 'pkg_a/Move.toml', 'pkg_a/sources/a.move', 'pkg_ab/Move.toml', @@ -30,8 +33,8 @@ describe('moveIngest package-ownership attribution', () => { const issues = output.consistencyIssues.filter((i) => i.code === 'empty-package-facts'); expect(issues.map((i) => i.details?.packageRoot).sort()).toEqual([ - '/repo/pkg_a', - '/repo/pkg_ab', + path.join(REPO_ROOT, 'pkg_a'), + path.join(REPO_ROOT, 'pkg_ab'), ]); // Exactly one owned .move file each - no prefix bleed between siblings. expect(issues.map((i) => i.details?.moveFileCount)).toEqual([1, 1]); @@ -41,7 +44,7 @@ describe('moveIngest package-ownership attribution', () => { // pkg_ab has no Move.toml, so its file belongs to NO package. With a bare // startsWith ownership test it would count towards pkg_a's empty-facts // diagnostics (moveFileCount 2 instead of 1). - const output = await runMoveIngestPhase(emptyFactsClient(), '/repo', [ + const output = await runMoveIngestPhase(emptyFactsClient(), REPO_ROOT, [ 'pkg_a/Move.toml', 'pkg_a/sources/a.move', 'pkg_ab/sources/x.move', @@ -49,7 +52,7 @@ describe('moveIngest package-ownership attribution', () => { const issues = output.consistencyIssues.filter((i) => i.code === 'empty-package-facts'); expect(issues).toHaveLength(1); - expect(issues[0].details?.packageRoot).toBe('/repo/pkg_a'); + expect(issues[0].details?.packageRoot).toBe(path.join(REPO_ROOT, 'pkg_a')); expect(issues[0].details?.moveFileCount).toBe(1); }); });