Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions ix-cli/src/cli/__tests__/ingest-retry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { describe, expect, it } from 'vitest';
import { isAbortError, isRetryableCommitConflict } from '../commands/ingest.js';

describe('isAbortError', () => {
it('detects AbortError and TimeoutError by name', () => {
const abort = Object.assign(new Error('The operation was aborted'), { name: 'AbortError' });
const timeout = Object.assign(new Error('signal timed out'), { name: 'TimeoutError' });
expect(isAbortError(abort)).toBe(true);
expect(isAbortError(timeout)).toBe(true);
});

it('detects an abort by message when the name is generic', () => {
expect(isAbortError(new Error('This operation was aborted'))).toBe(true);
});

it('is false for ordinary errors', () => {
expect(isAbortError(new Error('500: internal error'))).toBe(false);
expect(isAbortError('write-write conflict')).toBe(false);
});
});

describe('isRetryableCommitConflict', () => {
it('still retries Arango lock conflicts and transport drops', () => {
expect(isRetryableCommitConflict('write-write conflict')).toBe(true);
expect(isRetryableCommitConflict(new Error('Error: 1200 timeout waiting to lock key'))).toBe(true);
expect(isRetryableCommitConflict(new Error('fetch failed'))).toBe(true);
expect(isRetryableCommitConflict(new Error('read ECONNRESET'))).toBe(true);
});

it('does NOT retry a deadline/timeout abort (the deadline must stop work)', () => {
const abort = Object.assign(new Error('The operation was aborted'), { name: 'AbortError' });
const timeout = Object.assign(new Error('signal timed out'), { name: 'TimeoutError' });
expect(isRetryableCommitConflict(abort)).toBe(false);
expect(isRetryableCommitConflict(timeout)).toBe(false);
});

it('does not retry a plain 500', () => {
expect(isRetryableCommitConflict(new Error('500: internal server error'))).toBe(false);
});
});
23 changes: 23 additions & 0 deletions ix-cli/src/cli/__tests__/map-auto-skip.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { afterEach, describe, expect, it } from 'vitest';
import { shouldSkipAutoMap } from '../commands/map.js';

describe('shouldSkipAutoMap', () => {
afterEach(() => { delete process.env.IX_AUTO_MAP_CLOUD; });

it('skips an automatic map against a remote backend', () => {
expect(shouldSkipAutoMap({ auto: true, cloudReady: true })).toBe(true);
});

it('never skips a manual map (auto=false), even against a remote backend', () => {
expect(shouldSkipAutoMap({ auto: false, cloudReady: true })).toBe(false);
});

it('never skips an automatic map against a local backend', () => {
expect(shouldSkipAutoMap({ auto: true, cloudReady: false })).toBe(false);
});

it('honors the IX_AUTO_MAP_CLOUD opt-in to allow remote auto-refresh', () => {
process.env.IX_AUTO_MAP_CLOUD = '1';
expect(shouldSkipAutoMap({ auto: true, cloudReady: true })).toBe(false);
});
});
94 changes: 94 additions & 0 deletions ix-cli/src/cli/__tests__/single-flight.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { mkdtempSync, rmSync, writeFileSync, readdirSync, existsSync } from 'node:fs';
import { join } from 'node:path';
import { tmpdir, hostname } from 'node:os';

import { acquireMapLock, isStaleForTest, lockPathForTest } from '../single-flight.js';

describe('acquireMapLock single-flight', () => {
let dir: string;

beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), 'ix-lock-'));
process.env.IX_LOCK_DIR = dir;
delete process.env.IX_MAP_LOCK_MAX_MS;
});

afterEach(() => {
delete process.env.IX_LOCK_DIR;
delete process.env.IX_MAP_LOCK_MAX_MS;
rmSync(dir, { recursive: true, force: true });
});

it('grants the lock to the first caller and denies a concurrent caller', () => {
const first = acquireMapLock('/work/repo', 'ix map /work/repo');
expect(first).not.toBeNull();

const second = acquireMapLock('/work/repo', 'ix map /work/repo');
expect(second).toBeNull(); // coalesce — a live holder owns it

first!.release();
});

it('re-grants after release', () => {
const first = acquireMapLock('/work/repo', 'm');
expect(first).not.toBeNull();
first!.release();

const again = acquireMapLock('/work/repo', 'm');
expect(again).not.toBeNull();
again!.release();
});

it('isolates different workspaces', () => {
const a = acquireMapLock('/work/a', 'm');
const b = acquireMapLock('/work/b', 'm');
expect(a).not.toBeNull();
expect(b).not.toBeNull(); // different key → different lockfile
a!.release();
b!.release();
});

it('release is idempotent and removes the lockfile', () => {
const h = acquireMapLock('/work/repo', 'm');
expect(readdirSync(dir).length).toBe(1);
h!.release();
h!.release(); // no throw
expect(readdirSync(dir).length).toBe(0);
});

it('steals a stale lock left by a dead holder', () => {
const path = lockPathForTest('/work/repo');
// A lock owned by a PID that cannot be alive on this host.
writeFileSync(path, JSON.stringify({ pid: 2 ** 30, host: hostname(), startedAt: Date.now(), label: 'dead' }));
const h = acquireMapLock('/work/repo', 'm');
expect(h).not.toBeNull(); // stale holder → stolen
h!.release();
});

it('steals a lock older than the max age even if the holder looks alive', () => {
process.env.IX_MAP_LOCK_MAX_MS = '1000';
const path = lockPathForTest('/work/repo');
writeFileSync(path, JSON.stringify({ pid: process.pid, host: hostname(), startedAt: Date.now() - 60_000, label: 'old' }));
const h = acquireMapLock('/work/repo', 'm');
expect(h).not.toBeNull(); // aged out → stolen
h!.release();
});

it('treats an unparseable lockfile as abandoned', () => {
const path = lockPathForTest('/work/repo');
writeFileSync(path, 'not json');
expect(isStaleForTest(path)).toBe(true);
const h = acquireMapLock('/work/repo', 'm');
expect(h).not.toBeNull();
h!.release();
});

it('does not leak across a missing lock dir (fail-open is best-effort)', () => {
// Sanity: a brand-new dir yields a clean acquire.
expect(existsSync(dir)).toBe(true);
const h = acquireMapLock('/work/fresh', 'm');
expect(h).not.toBeNull();
h!.release();
});
});
16 changes: 14 additions & 2 deletions ix-cli/src/cli/commands/ingest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@
return maxLineLength >= MINIFIED_MAX_LINE_THRESHOLD || (lineCount <= 50 && averageLineLength >= MINIFIED_AVG_LINE_THRESHOLD);
}

function stripChunkOps(patch: GraphPatchPayload): GraphPatchPayload {

Check warning on line 255 in ix-cli/src/cli/commands/ingest.ts

View workflow job for this annotation

GitHub Actions / Lint & Typecheck

'stripChunkOps' is defined but never used. Allowed unused vars must match /^_/u
return {
...patch,
ops: patch.ops.filter(op => {
Expand Down Expand Up @@ -287,7 +287,19 @@
'econnrefused',
];

// An abort from the shared wall-clock deadline (or a per-request timeout) must
// NOT be retried — the whole point of the deadline is to stop work. Retrying
// here would defeat it and add 6 more backed-off requests to an already-late
// run. AbortSignal.timeout raises a TimeoutError; an aborted deadline raises an
// AbortError; both also stringify with "aborted".
export function isAbortError(err: unknown): boolean {
const name = (err as { name?: string } | null)?.name;
if (name === "AbortError" || name === "TimeoutError") return true;
return String(err).toLowerCase().includes("aborted");
}

export function isRetryableCommitConflict(err: unknown): boolean {
if (isAbortError(err)) return false;
const message = String(err).toLowerCase();
return COMMIT_CONFLICT_RETRY_PATTERNS.some(pattern => message.includes(pattern));
}
Expand Down Expand Up @@ -399,14 +411,14 @@

export async function ingestFiles(
path: string,
opts: { recursive?: boolean; force?: boolean; format: string; root?: string; debug?: boolean; printSummary?: boolean; suppressOutput?: boolean; lang?: string; mapMode?: boolean }
opts: { recursive?: boolean; force?: boolean; format: string; root?: string; debug?: boolean; printSummary?: boolean; suppressOutput?: boolean; lang?: string; mapMode?: boolean; deadlineSignal?: AbortSignal }
): Promise<void> {
const debug = opts.debug || process.env.IX_DEBUG === '1';
const mapMode = opts.mapMode === true;
const trueStart = performance.now();

const [{ parseFile, resolveEdges, isGrammarSupported }, { buildPatchWithResolution, fileNodeId, symbolNodeId }, { languageFromPath }] = await loadIngestionModules();

Check warning on line 420 in ix-cli/src/cli/commands/ingest.ts

View workflow job for this annotation

GitHub Actions / Lint & Typecheck

'buildPatchWithResolution' is assigned a value but never used. Allowed unused vars must match /^_/u

Check warning on line 420 in ix-cli/src/cli/commands/ingest.ts

View workflow job for this annotation

GitHub Actions / Lint & Typecheck

'resolveEdges' is assigned a value but never used. Allowed unused vars must match /^_/u

Check warning on line 420 in ix-cli/src/cli/commands/ingest.ts

View workflow job for this annotation

GitHub Actions / Lint & Typecheck

'parseFile' is assigned a value but never used. Allowed unused vars must match /^_/u
const moduleLoadMs = Math.round(performance.now() - trueStart);

Check warning on line 421 in ix-cli/src/cli/commands/ingest.ts

View workflow job for this annotation

GitHub Actions / Lint & Typecheck

'moduleLoadMs' is assigned a value but never used. Allowed unused vars must match /^_/u


const resolvedPath = nodePath.isAbsolute(path)
Expand Down Expand Up @@ -576,7 +588,7 @@
process.stderr.write(`[multi-repo] system "${detectedSystem!.name}" (${systemId}) members=${detectedSystem!.members.join(', ')} packages=${Object.keys(packageRegistry).length} declaredDeps=${depCount}\n`);
}

const client = new IxClient(getEndpoint());
const client = new IxClient(getEndpoint(), opts.deadlineSignal);

// Schema-version check forces a clean re-ingest when the backend's graph
// format has changed in a way that invalidates existing node IDs (e.g. the
Expand Down
68 changes: 65 additions & 3 deletions ix-cli/src/cli/commands/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,32 @@ import { formatFetchError } from "../errors.js";
import { ingestFiles } from "./ingest.js";
import { detectSystem } from "../system.js";
import { getRemoteRunner, isCloudReady } from "../remote.js";
import { acquireMapLock } from "../single-flight.js";

// Hard wall-clock budget for a single `ix map`. Past this, the shared deadline
// signal aborts every in-flight request and the command exits, so a single
// invocation can never grind for hours against an unhealthy backend. Tunable
// via IX_MAP_DEADLINE_MS; 0 disables the budget.
const DEFAULT_MAP_DEADLINE_MS = 15 * 60 * 1000;

function mapDeadlineSignal(): AbortSignal | undefined {
const raw = process.env.IX_MAP_DEADLINE_MS;
const budget = raw !== undefined ? Number.parseInt(raw, 10) : DEFAULT_MAP_DEADLINE_MS;
if (!Number.isFinite(budget) || budget <= 0) return undefined; // disabled
return AbortSignal.timeout(budget);
}

// Whether an automatically-triggered map should be skipped. Background refresh
// (the editor/agent hooks that re-map on change) is a local-only convenience:
// against a remote backend it would push a write on every change from every
// client, so it is skipped there and remote ingestion stays deliberate. Manual
// `ix map` never sets IX_AUTO_MAP, so it is never skipped. IX_AUTO_MAP_CLOUD=1
// opts the automatic path back in for users who do want remote auto-refresh.
export function shouldSkipAutoMap(opts: { auto: boolean; cloudReady: boolean }): boolean {
if (!opts.auto || !opts.cloudReady) return false;
if (process.env.IX_AUTO_MAP_CLOUD === "1") return false;
return true;
}

export interface MapRegion {
id: string;
Expand Down Expand Up @@ -133,11 +159,47 @@ Examples:
)
.action(async (pathArg: string | undefined, opts: { format: string; level?: string; minConfidence: string; maxItems: string; allItems?: boolean; sort: string; graph?: boolean; list?: boolean; full?: boolean; verbose?: boolean; silent?: boolean }) => {
const cwd = pathArg ? resolve(pathArg) : process.cwd();

const silent = opts.silent === true || opts.format === "silent";

// Single-flight: refuse to stack. Background refresh can fire `ix map`
// repeatedly (e.g. once per change); if a map is slow or the backend is
// unhealthy, those invocations would otherwise pile up and run concurrently
// against the backend. The first map for a workspace holds the lock; any
// concurrent one coalesces and exits 0 here. The lock auto-releases on
// process exit (see single-flight.ts) and a stale lock from a crashed map
// is stolen, so this never wedges.
const mapLock = acquireMapLock(cwd, `ix map ${cwd}`);
if (!mapLock) {
if (!silent && opts.format !== "json" && opts.format !== "llm") {
process.stderr.write(chalk.dim(" Another ix map is already running for this workspace — skipping.\n"));
}
return; // coalesce; the in-flight map will refresh the graph
}

// Background refresh is a local-only convenience. When invoked
// automatically (IX_AUTO_MAP=1, set by the editor/agent hooks) against a
// remote backend, skip: a remote graph should be fed deliberately, not by
// a write on every change from every client. Manual `ix map` is never
// skipped; opt the automatic path back in with IX_AUTO_MAP_CLOUD=1.
const autoMap = process.env.IX_AUTO_MAP === "1";
const cloudReady = await isCloudReady();
if (shouldSkipAutoMap({ auto: autoMap, cloudReady })) {
if (!silent && opts.format !== "json" && opts.format !== "llm") {
process.stderr.write(chalk.dim(" Skipping automatic map: active backend is remote (run `ix map` manually to refresh it).\n"));
}
return; // lock releases on process exit
}

// Shared wall-clock deadline applied to every backend request this command
// makes (ingest + map), so the whole operation is bounded even if the
// backend stalls on individual long per-request timeouts.
const deadlineSignal = mapDeadlineSignal();

// Auto-detect a multi-repo system (>= 2 child repo roots). When present we
// scope the map to its system_id; otherwise it's an ordinary single-repo map.
const systemId = detectSystem(cwd)?.systemId;

const silent = opts.silent === true || opts.format === "silent";
// json and llm are machine formats: suppress progress chatter and route
// ingestion through the quiet path so stdout carries only the result.
const machineFormat = opts.format === "json" || opts.format === "llm";
Expand Down Expand Up @@ -166,7 +228,6 @@ Examples:
// The local backend bootstrap below only runs on the local path —
// cloud ingestion doesn't require a local Ix backend.
const ingestStart = performance.now();
const cloudReady = await isCloudReady();
if (cloudReady) {
const runner = getRemoteRunner()!; // isCloudReady guarantees non-null
try {
Expand Down Expand Up @@ -195,6 +256,7 @@ Examples:
printSummary: false,
suppressOutput: true,
mapMode: true,
deadlineSignal,
});
} catch (err: any) {
emitError(formatFetchError(err));
Expand All @@ -204,7 +266,7 @@ Examples:
}
const ingestMs = Math.round(performance.now() - ingestStart);

const client = new IxClient(getEndpoint());
const client = new IxClient(getEndpoint(), deadlineSignal);

const mapBarWidth = 25;
const mapStart = performance.now();
Expand Down
Loading
Loading