mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
fix(move): harden real-repo Aptos ingestion
This commit is contained in:
parent
8127b75c0f
commit
7abb2c9bb5
7 changed files with 91 additions and 13 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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, '/');
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<unknown>((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) => {
|
||||
|
|
|
|||
19
gitnexus/test/unit/move/constants.test.ts
Normal file
19
gitnexus/test/unit/move/constants.test.ts
Normal file
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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> = {}): MoveFlowClient {
|
||||
return {
|
||||
facts: async () => ({}),
|
||||
|
|
@ -24,7 +27,7 @@ function makeClient(overrides: Partial<MoveFlowClient> = {}): 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<ReturnType<typeof runPhase>>) {
|
||||
|
|
@ -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: [],
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue