vvzvlad/gitmost
1
0
Fork 0
Code Issues 14 Pull Requests 12 Packages Projects Releases Activity

Address review on agent-roles catalog: changelog, docs, BadGateway on body-read abort

Browse Source 
- CHANGELOG: document the importable multilingual agent-roles catalog under
  [Unreleased] (browse/import/update, 4 new endpoints, source column, the new
  AI_AGENT_ROLES_CATALOG_URL env var) (#222).
- Fix importFromCatalog docstring: a role is skipped only on source.slug AND
  source.language; another language of the same slug still imports.
- Provider: map a timeout/abort (or any failure) during the response-BODY read
  to a logged BadGatewayException, so a slow/dripping source yields a 502, not a
  generic 500. Existing too-large BadGateway cases are rethrown as-is.
- Service: inject a Nest Logger and log the root cause (with workspaceId/
  bundleId/slug) on a non-23505 insert error during import.
- Modal: hoist the duplicated i18n base-subtag into a single baseLang const.
- Tests: AbortError body-read -> BadGateway; null-body text() fallback (under
  and over cap); invalid-JSON and malformed-index BadGateway; non-23505 import
  error -> generic message + logged root cause.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-26 23:15:45 +03:00
parent 8be8279809
commit 50e79275e1
6 changed files with 193 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
CHANGELOG.md
View File

@@ -28,6 +28,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
answer was cut off and builds on it instead of restarting; the rest of the
queue still flushes normally afterward. (#198)
- **Importable multilingual agent-roles catalog.** Admins can browse a curated
catalog of agent roles, grouped into bundles and offered in several languages,
and import the ones they want into the workspace (with skip-or-rename handling
for name collisions); the same role in a different language imports as a
separate install. An imported role remembers its catalog origin and offers a
one-click update when the catalog ships a newer revision. Backed by four new
admin endpoints — `POST /ai-chat/roles/catalog` (browse bundles),
`/catalog/bundle` (read one bundle's roles), `/import`, and
`/update-from-catalog` — and a new `source` column linking a role to its
catalog slug/language/version. The catalog source is configurable via the new
`AI_AGENT_ROLES_CATALOG_URL` env var (an `http(s)://` base URL fetches it
remotely; otherwise a local directory; empty defaults to the in-repo
`agent-roles-catalog/` folder — see `.env.example`). (#222)
## [0.94.0] - 2026-06-26
This release makes AI chat durable and fast: assistant turns are persisted to

19
apps/client/src/features/workspace/components/settings/components/ai-agent-roles-catalog-modal.tsx
View File

@@ -55,14 +55,16 @@ export default function AiAgentRolesCatalogModal({
}: AiAgentRolesCatalogModalProps) {
const { t, i18n } = useTranslation();
// The user's i18n base subtag (e.g. "ru-RU" => "ru"); the preferred catalog
// language both when seeding and when reconciling against offered languages.
const baseLang = (i18n.language || "en").split("-")[0].toLowerCase();
// Fetch the catalog only while the modal is open. `language` drives both the
// catalog query (bundle names) and bundle reads (role content). Seed it
// synchronously from the i18n base subtag (e.g. "ru-RU" => "ru") so the first
// fetch already uses the user's language; the effect below still reconciles
// against the catalog's offered languages once they load.
const [language, setLanguage] = useState<string>(
() => (i18n.language || "en").split("-")[0].toLowerCase(),
);
// synchronously from the base subtag so the first fetch already uses the
// user's language; the effect below still reconciles against the catalog's
// offered languages once they load.
const [language, setLanguage] = useState<string>(() => baseLang);
const catalogQuery = useAiRoleCatalogQuery(language || "en", opened);
// On name conflict: Skip (default) or Rename to a free " (N)" name.
@@ -82,9 +84,8 @@ export default function AiAgentRolesCatalogModal({
useEffect(() => {
if (!languages || languages.length === 0) return;
if (language && languages.includes(language)) return;
const base = (i18n.language || "en").split("-")[0].toLowerCase();
const preferred = languages.includes(base)
? base
const preferred = languages.includes(baseLang)
? baseLang
: languages.includes("en")
? "en"
: languages[0];

37
apps/server/src/core/ai-chat/roles/ai-agent-roles.service.spec.ts
View File

@@ -2,6 +2,7 @@ import {
BadGatewayException,
BadRequestException,
ConflictException,
Logger,
} from '@nestjs/common';
import { AiAgentRolesService } from './ai-agent-roles.service';
import type { AiAgentRole } from '@docmost/db/types/entity.types';
@@ -645,6 +646,42 @@ describe('AiAgentRolesService guards', () => {
{ slug: 'a', message: 'A role with this name already exists' },
]);
});
it('non-unique insert error => generic message, root cause logged, import continues', async () => {
const logSpy = jest
.spyOn(Logger.prototype, 'error')
.mockImplementation(() => undefined);
try {
const { service, repo } = makeImportService({
bundleRoles: [
catalogRole({ slug: 'a', name: 'A' }),
catalogRole({ slug: 'b', name: 'B' }),
],
indexRoles: [
{ slug: 'a', version: 1 },
{ slug: 'b', version: 1 },
],
});
// A non-23505 failure (e.g. a not-null violation) on the first insert.
const boom = Object.assign(new Error('null value in column'), {
code: '23502',
});
repo.insert
.mockRejectedValueOnce(boom)
.mockImplementationOnce((v) => Promise.resolve(makeRow(v)));
const res = await service.importFromCatalog('ws-1', 'u1', dto());
// The generic (non-409) user-facing message; the second role still imports.
expect(res.created).toBe(1);
expect(res.errors).toEqual([
{ slug: 'a', message: 'Failed to import role' },
]);
// The root cause was logged with the slug for diagnosis.
expect(logSpy).toHaveBeenCalledTimes(1);
expect(String(logSpy.mock.calls[0][0])).toContain('slug=a');
} finally {
logSpy.mockRestore();
}
});
});
describe('updateFromCatalog', () => {

28
apps/server/src/core/ai-chat/roles/ai-agent-roles.service.ts
View File

@@ -3,6 +3,7 @@ import {
BadRequestException,
ConflictException,
Injectable,
Logger,
} from '@nestjs/common';
import { AiAgentRoleRepo } from '@docmost/db/repos/ai-agent-roles/ai-agent-roles.repo';
import { AiAgentRole } from '@docmost/db/types/entity.types';
@@ -71,6 +72,8 @@ export interface AgentRolePickerView {
*/
@Injectable()
export class AiAgentRolesService {
private readonly logger = new Logger(AiAgentRolesService.name);
constructor(
private readonly repo: AiAgentRoleRepo,
private readonly catalog: AiAgentRolesCatalogProvider,
@@ -264,12 +267,16 @@ export class AiAgentRolesService {
}
/**
* Import a bundle's roles into the workspace. Roles whose `source.slug` is
* already installed are skipped (updates are a separate action). A name
* collision with an existing role is either skipped or imported under a free
* " (N)" name, per `dto.conflict`. Inserts run sequentially (the repo exposes
* no batch insert and the volume is tiny); a unique-name race still surfaces
* as an error entry rather than aborting the whole import.
* Import a bundle's roles into the workspace. A role is "already installed"
* (and thus skipped — updates are a separate action) only when an existing
* role matches BOTH its `source.slug` AND `source.language`: this is a
* multilingual catalog, so a different language of the same slug (e.g. the
* `ru` variant of a slug already installed as `en`) is a SEPARATE install and
* still imports. A name collision with an existing role is either skipped or
* imported under a free " (N)" name, per `dto.conflict`. Inserts run
* sequentially (the repo exposes no batch insert and the volume is tiny); a
* unique-name race still surfaces as an error entry rather than aborting the
* whole import.
*/
async importFromCatalog(
workspaceId: string,
@@ -368,6 +375,15 @@ export class AiAgentRolesService {
takenNames.add(name.toLowerCase());
installedKeys.add(installKey);
} catch (err) {
// A unique-name race is expected and self-explanatory (it becomes a
// friendly per-role error). Any OTHER insert failure is unexpected, so
// log the root cause with enough context to diagnose it — the
// user-facing message is deliberately generic.
if (!isUniqueViolation(err)) {
this.logger.error(
`Failed to import catalog role (workspaceId=${workspaceId} bundleId=${dto.bundleId} slug=${role.slug}): ${err instanceof Error ? err.stack ?? err.message : String(err)}`,
);
}
errors.push({ slug: role.slug, message: importErrorMessage(err) });
}
}

87
apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.spec.ts
View File

@@ -131,18 +131,29 @@ describe('AiAgentRolesCatalogProvider (local fixtures)', () => {
});
}
/** A ReadableStream whose first read rejects (e.g. a mid-body AbortError). */
function errorStream(err: Error): ReadableStream<Uint8Array> {
return new ReadableStream<Uint8Array>({
pull() {
throw err;
},
cancel() {},
});
}
function mockResponse(opts: {
ok?: boolean;
status?: number;
headers?: Record<string, string>;
body: ReadableStream<Uint8Array> | null;
text?: string;
}): Response {
return {
ok: opts.ok ?? true,
status: opts.status ?? 200,
headers: { get: (k: string) => opts.headers?.[k.toLowerCase()] ?? null },
body: opts.body,
text: async () => 'unused',
text: async () => opts.text ?? 'unused',
} as unknown as Response;
}
@@ -212,6 +223,80 @@ describe('AiAgentRolesCatalogProvider (local fixtures)', () => {
const index = await provider.fetchIndex();
expect(index.bundles[0].id).toBe('general');
});
it('body read aborts mid-stream (AbortError) => BadGateway (not a generic 500)', async () => {
// The 10s timer aborts the whole request; on a slow/dripping source the
// body read (reader.read()) rejects with an AbortError AFTER fetch()
// resolved. The provider must map that to BadGateway, not let it escape.
const abortErr = Object.assign(new Error('The operation was aborted'), {
name: 'AbortError',
});
global.fetch = jest
.fn()
.mockResolvedValue(mockResponse({ body: errorStream(abortErr) })) as never;
const provider = makeProvider('https://catalog.example.com');
await expect(provider.fetchIndex()).rejects.toBeInstanceOf(
BadGatewayException,
);
});
it('null body (no readable stream) => response.text() fallback parses', async () => {
const json = JSON.stringify({
schemaVersion: 1,
bundles: [
{
id: 'general',
name: { en: 'General' },
languages: ['en'],
roles: [{ slug: 'researcher', version: 2 }],
},
],
});
global.fetch = jest
.fn()
.mockResolvedValue(mockResponse({ body: null, text: json })) as never;
const provider = makeProvider('https://catalog.example.com');
const index = await provider.fetchIndex();
expect(index.bundles[0].id).toBe('general');
});
it('null body + text() over the cap => BadGateway (too large)', async () => {
const oversized = 'a'.repeat(1_000_001);
global.fetch = jest
.fn()
.mockResolvedValue(
mockResponse({ body: null, text: oversized }),
) as never;
const provider = makeProvider('https://catalog.example.com');
await expect(provider.fetchIndex()).rejects.toBeInstanceOf(
BadGatewayException,
);
});
it('invalid JSON body => BadGateway (parse failure)', async () => {
const body = streamOf([new TextEncoder().encode('{not valid json')]);
global.fetch = jest
.fn()
.mockResolvedValue(mockResponse({ body })) as never;
const provider = makeProvider('https://catalog.example.com');
await expect(provider.fetchIndex()).rejects.toBeInstanceOf(
BadGatewayException,
);
});
it('malformed index.json (valid JSON, wrong shape) => BadGateway', async () => {
// Parses as JSON but fails isCatalogIndex (schemaVersion not a number).
const body = streamOf([
new TextEncoder().encode(
JSON.stringify({ schemaVersion: 'x', bundles: [] }),
),
]);
global.fetch = jest
.fn()
.mockResolvedValue(mockResponse({ body })) as never;
const provider = makeProvider('https://catalog.example.com');
await expect(provider.fetchIndex()).rejects.toThrow(/malformed/i);
});
});
describe('path-traversal / SSRF guard (^[a-z0-9-]+$)', () => {

32
apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.ts
View File

@@ -156,15 +156,31 @@ export class AiAgentRolesCatalogProvider {
throw new BadGatewayException('Agent roles catalog file is too large');
}
// Bound the actual read: a missing/lying Content-Length is caught here.
if (response.body) {
return await readStreamCapped(response.body, MAX_BYTES);
// The 10s timer aborts the WHOLE request, so a slow/dripping hostile
// source rejects reader.read() (or response.text()) with an AbortError
// mid-body. Map that — and any other read failure — to a logged
// BadGateway so the admin endpoint returns 502 (not a generic 500). The
// cap's own BadGateway is rethrown as-is (no double-wrap).
try {
if (response.body) {
return await readStreamCapped(response.body, MAX_BYTES);
}
// Edge: no readable stream — fall back to a buffered read + length check.
const text = await response.text();
if (text.length > MAX_BYTES) {
throw new BadGatewayException('Agent roles catalog file is too large');
}
return text;
} catch (err) {
if (err instanceof BadGatewayException) throw err;
const reason = shortError(err);
this.logger.error(
`Agent roles catalog body read failed (${rel}): ${reason}`,
);
throw new BadGatewayException(
`Agent roles catalog is unavailable: ${reason}`,
);
}
// Edge: no readable stream — fall back to a buffered read + length check.
const text = await response.text();
if (text.length > MAX_BYTES) {
throw new BadGatewayException('Agent roles catalog file is too large');
}
return text;
} finally {
clearTimeout(timer);
}