fix(group): surface friendly error when group name not found (#903 regression test) (#989)

* fix(group): surface friendly error when group name not found

Squashed commits:
- test(csharp): add #903 regression — parse completeness for single-file C# repo
- fix(group): add GroupNotFoundError guard to groupList + re-throw tests for groupQuery/groupStatus
- fix(test): restore section comments in csharp.test.ts stripped during rebase

* fix(group): catch GroupNotFoundError explicitly in groupContext and groupImpact
This commit is contained in:
Sam Fakhreddine 2026-04-21 08:52:36 -06:00 committed by GitHub
parent ff4ae89aaa
commit 95a38c7e2d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 247 additions and 19 deletions

View file

@ -89,10 +89,25 @@ export function parseGroupConfig(yamlContent: string): GroupConfig {
};
}
export class GroupNotFoundError extends Error {
constructor(public readonly groupName: string) {
super(`Group "${groupName}" not found`);
this.name = 'GroupNotFoundError';
}
}
export async function loadGroupConfig(groupDir: string): Promise<GroupConfig> {
const fsp = await import('node:fs/promises');
const path = await import('node:path');
const yamlPath = path.join(groupDir, 'group.yaml');
const content = await fsp.readFile(yamlPath, 'utf-8');
let content: string;
try {
content = await fsp.readFile(yamlPath, 'utf-8');
} catch (err) {
if ((err as NodeJS.ErrnoException).code === 'ENOENT') {
throw new GroupNotFoundError(path.basename(groupDir));
}
throw err;
}
return parseGroupConfig(content);
}

View file

@ -15,7 +15,7 @@ import type {
OutOfScopeLink,
} from './types.js';
import type { GroupRepoHandle, GroupToolPort } from './service.js';
import { loadGroupConfig } from './config-parser.js';
import { GroupNotFoundError, loadGroupConfig } from './config-parser.js';
import {
fileMatchesServicePrefix,
normalizeServicePrefix,
@ -329,6 +329,8 @@ export async function runGroupImpact(
try {
config = await loadGroupConfig(groupDir);
} catch (e) {
if (e instanceof GroupNotFoundError)
return { error: `Group "${name}" not found. Run group_list to see configured groups.` };
return { error: e instanceof Error ? e.message : String(e) };
}
@ -344,9 +346,6 @@ export async function runGroupImpact(
minConfidence,
};
// Single shared deadline for Phase 1 (local walk) + Phase 2 (bridge fan-out).
// Phase 1 still gets the full budget; Phase 2 only uses whatever wall-clock
// time is left, so total work cannot exceed `timeoutMs`.
const deadline = Date.now() + Math.max(0, timeoutMs);
const { value: local, timedOut: localTimedOut } = await safeLocalImpact(
@ -357,7 +356,7 @@ export async function runGroupImpact(
);
if (localTimedOut) {
const base = local as Record<string, unknown>;
const _base = local as Record<string, unknown>;
return {
local,
group: name,
@ -464,7 +463,6 @@ export async function runGroupImpact(
continue;
}
if (!repoInSubgroup(n.neighborRepo, subgroup)) {
// CrossLink convention: consumer -> provider
outOfScope.push({
from: direction === 'upstream' ? n.neighborRepo : repoPath,
to: direction === 'upstream' ? repoPath : n.neighborRepo,

View file

@ -6,7 +6,7 @@
import fsp from 'node:fs/promises';
import path from 'node:path';
import { checkStaleness } from '../git-staleness.js';
import { loadGroupConfig } from './config-parser.js';
import { GroupNotFoundError, loadGroupConfig } from './config-parser.js';
import {
fileMatchesServicePrefix,
normalizeServicePrefix,
@ -221,7 +221,14 @@ export class GroupService {
return { groups };
}
const groupDir = getGroupDir(getDefaultGitnexusDir(), name);
const config = await loadGroupConfig(groupDir);
let config: GroupConfig;
try {
config = await loadGroupConfig(groupDir);
} catch (err) {
if (err instanceof GroupNotFoundError)
return { error: `Group "${name}" not found. Run group_list to see configured groups.` };
throw err;
}
return {
name: config.name,
description: config.description,
@ -234,7 +241,14 @@ export class GroupService {
const name = String(params.name ?? '').trim();
if (!name) return { error: 'name is required' };
const groupDir = getGroupDir(getDefaultGitnexusDir(), name);
const config = await loadGroupConfig(groupDir);
let config: GroupConfig;
try {
config = await loadGroupConfig(groupDir);
} catch (err) {
if (err instanceof GroupNotFoundError)
return { error: `Group "${name}" not found. Run group_list to see configured groups.` };
throw err;
}
const result = await syncGroup(config, {
groupDir,
exactOnly: Boolean(params.exactOnly),
@ -313,6 +327,14 @@ export class GroupService {
try {
config = await loadGroupConfig(groupDir);
} catch (e) {
if (e instanceof GroupNotFoundError)
return {
group: name,
target: target || uid,
service: servicePrefix,
error: `Group "${name}" not found. Run group_list to see configured groups.`,
results: [],
};
return {
group: name,
target: target || uid,
@ -326,9 +348,6 @@ export class GroupService {
repoInSubgroup(repoPath, subgroup, subgroupExact),
);
// Per-repo work is independent (each repo opens its own DB handle and the
// group-level result preserves repo iteration order via the indexed map).
// Errors are caught per repo so one slow/failed member does not block the rest.
const results: GroupContextResult['results'] = await Promise.all(
memberEntries.map(async ([repoPath, registryName]) => {
try {
@ -384,14 +403,19 @@ export class GroupService {
const subgroup = typeof params.subgroup === 'string' ? params.subgroup : undefined;
const subgroupExact = params.subgroupExact === true;
const groupDir = getGroupDir(getDefaultGitnexusDir(), name);
const config = await loadGroupConfig(groupDir);
let config: GroupConfig;
try {
config = await loadGroupConfig(groupDir);
} catch (err) {
if (err instanceof GroupNotFoundError)
return { error: `Group "${name}" not found. Run group_list to see configured groups.` };
throw err;
}
const memberEntries = Object.entries(config.repos).filter(([repoPath]) =>
repoInSubgroup(repoPath, subgroup, subgroupExact),
);
// Per-repo query is independent; run them concurrently and isolate
// failures so one slow/failed member does not block the rest.
const perRepo = await Promise.all(
memberEntries.map(async ([repoPath, registryName]) => {
try {
@ -436,7 +460,14 @@ export class GroupService {
const name = String(params.name ?? '').trim();
if (!name) return { error: 'name is required' };
const groupDir = getGroupDir(getDefaultGitnexusDir(), name);
const config = await loadGroupConfig(groupDir);
let config: GroupConfig;
try {
config = await loadGroupConfig(groupDir);
} catch (err) {
if (err instanceof GroupNotFoundError)
return { error: `Group "${name}" not found. Run group_list to see configured groups.` };
throw err;
}
const registry = await readContractRegistry(groupDir);
const repoStatuses: Record<

View file

@ -0,0 +1,17 @@
namespace Demo;
public class Greeter
{
public string Greet(string name) => $"Hello, {name}!";
public static void Main(string[] args)
{
var g = new Greeter();
System.Console.WriteLine(g.Greet("world"));
}
}
public interface IFoo
{
void Bar();
}

View file

@ -124,10 +124,10 @@ describe('C# ambiguous symbol resolution', () => {
// The key invariant: no edge points to Other/
if (extends_[0].targetFilePath) {
expect(extends_[0].targetFilePath).not.toMatch(/Other\//);
expect(extends_[0].targetFilePath).not.toContain('Other/');
}
if (implements_[0].targetFilePath) {
expect(implements_[0].targetFilePath).not.toMatch(/Other\//);
expect(implements_[0].targetFilePath).not.toContain('Other/');
}
});
});
@ -2048,3 +2048,77 @@ describe('C# interface-to-interface heritage', () => {
expect(implements_.length).toBe(4);
});
});
// ---------------------------------------------------------------------------
// C# parse completeness regression (#903)
// ---------------------------------------------------------------------------
describe('C# parse completeness (#903 regression)', () => {
let result: PipelineResult;
beforeAll(async () => {
result = await runPipelineFromRepo(path.join(FIXTURES, 'csharp-hello'), () => {});
}, 60000);
it('parse phase completes without error (no crash)', () => {
expect(result).toBeDefined();
expect(result.graph).toBeDefined();
});
it('emits Class node for Greeter', () => {
const classes = getNodesByLabel(result, 'Class');
expect(classes).toContain('Greeter');
});
it('emits Interface node for IFoo', () => {
const interfaces = getNodesByLabel(result, 'Interface');
expect(interfaces).toContain('IFoo');
});
it('emits Method nodes for Greet, Main, and Bar', () => {
const methods = getNodesByLabel(result, 'Method');
expect(methods).toContain('Greet');
expect(methods).toContain('Main');
expect(methods).toContain('Bar');
});
it('Greet has parameterCount=1 and returnType=string', () => {
const methods = getNodesByLabelFull(result, 'Method');
const greet = methods.find((m) => m.name === 'Greet');
expect(greet).toBeDefined();
expect(greet!.properties.parameterCount).toBe(1);
expect(greet!.properties.returnType).toBe('string');
expect(greet!.properties.visibility).toBe('public');
});
it('Main has parameterCount=1 and isStatic=true', () => {
const methods = getNodesByLabelFull(result, 'Method');
const main = methods.find((m) => m.name === 'Main');
expect(main).toBeDefined();
expect(main!.properties.parameterCount).toBe(1);
expect(main!.properties.isStatic).toBe(true);
expect(main!.properties.visibility).toBe('public');
});
it('Bar is abstract with parameterCount=0 and returnType=void', () => {
const methods = getNodesByLabelFull(result, 'Method');
const bar = methods.find((m) => m.name === 'Bar');
expect(bar).toBeDefined();
expect(bar!.properties.parameterCount).toBe(0);
expect(bar!.properties.isAbstract).toBe(true);
expect(bar!.properties.returnType).toBe('void');
});
it('emits HAS_METHOD edges linking Greeter to its methods', () => {
const hasMethod = getRelationships(result, 'HAS_METHOD');
const targets = edgeSet(hasMethod);
expect(targets).toContain('Greeter → Greet');
expect(targets).toContain('Greeter → Main');
});
it('emits HAS_METHOD edge linking IFoo to Bar', () => {
const hasMethod = getRelationships(result, 'HAS_METHOD');
const targets = edgeSet(hasMethod);
expect(targets).toContain('IFoo → Bar');
});
});

View file

@ -0,0 +1,93 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
const loadGroupConfigMock = vi.fn();
const getGroupDirMock = vi.fn(() => '/fake/.gitnexus/groups/missing');
const getDefaultGitnexusDirMock = vi.fn(() => '/fake/.gitnexus');
const readContractRegistryMock = vi.fn(() => null);
const listGroupsMock = vi.fn(() => []);
const syncGroupMock = vi.fn();
vi.mock('../../src/core/group/config-parser.js', async () => {
const { GroupNotFoundError } = await vi.importActual<
typeof import('../../src/core/group/config-parser.js')
>('../../src/core/group/config-parser.js');
return { loadGroupConfig: loadGroupConfigMock, GroupNotFoundError };
});
vi.mock('../../src/core/group/storage.js', () => ({
getDefaultGitnexusDir: getDefaultGitnexusDirMock,
getGroupDir: getGroupDirMock,
readContractRegistry: readContractRegistryMock,
listGroups: listGroupsMock,
}));
vi.mock('../../src/core/group/sync.js', () => ({ syncGroup: syncGroupMock }));
vi.mock('../../src/core/git-staleness.js', () => ({ checkStaleness: vi.fn() }));
describe('GroupService — missing group error handling', () => {
let GroupService: typeof import('../../src/core/group/service.js').GroupService;
let GroupNotFoundError: typeof import('../../src/core/group/config-parser.js').GroupNotFoundError;
let service: InstanceType<typeof import('../../src/core/group/service.js').GroupService>;
const stubPort = {
resolveRepo: vi.fn(),
impact: vi.fn(),
query: vi.fn(),
impactByUid: vi.fn(),
contextByUid: vi.fn(),
};
beforeEach(async () => {
vi.resetModules();
loadGroupConfigMock.mockReset();
({ GroupService } = await import('../../src/core/group/service.js'));
({ GroupNotFoundError } = await import('../../src/core/group/config-parser.js'));
service = new GroupService(stubPort as never);
loadGroupConfigMock.mockRejectedValue(new GroupNotFoundError('missing'));
});
it('groupSync returns friendly error for missing group', async () => {
const result = await service.groupSync({ name: 'missing' });
expect(result).toEqual({
error: 'Group "missing" not found. Run group_list to see configured groups.',
});
});
it('groupQuery returns friendly error for missing group', async () => {
const result = await service.groupQuery({ name: 'missing', query: 'auth' });
expect(result).toEqual({
error: 'Group "missing" not found. Run group_list to see configured groups.',
});
});
it('groupStatus returns friendly error for missing group', async () => {
const result = await service.groupStatus({ name: 'missing' });
expect(result).toEqual({
error: 'Group "missing" not found. Run group_list to see configured groups.',
});
});
it('groupSync re-throws non-ENOENT errors', async () => {
loadGroupConfigMock.mockRejectedValue(new Error('YAML parse error'));
await expect(service.groupSync({ name: 'bad-yaml' })).rejects.toThrow('YAML parse error');
});
it('groupQuery re-throws non-ENOENT errors', async () => {
loadGroupConfigMock.mockRejectedValue(new Error('YAML parse error'));
await expect(service.groupQuery({ name: 'bad-yaml', query: 'auth' })).rejects.toThrow(
'YAML parse error',
);
});
it('groupStatus re-throws non-ENOENT errors', async () => {
loadGroupConfigMock.mockRejectedValue(new Error('YAML parse error'));
await expect(service.groupStatus({ name: 'bad-yaml' })).rejects.toThrow('YAML parse error');
});
it('groupList returns friendly error for missing group', async () => {
const result = await service.groupList({ name: 'missing' });
expect(result).toEqual({
error: 'Group "missing" not found. Run group_list to see configured groups.',
});
});
});