refactor(agent-roles-catalog): YAML catalog with block-scalar instructions (#229) #231

Ghost · 2026-06-27T06:37:00+03:00

Ghost commented

2026-06-27 06:37:00 +03:00

Closes #229.

Каталог ролей переведён на YAML: index.yaml + bundles/<id>/<lang>.yaml, instructions — литеральный блок-скаляр (|-) → построчные диффы, редактируется как обычный текст. Конвертация программная с deepStrictEqual round-trip против старого JSON (контент байт-в-байт, версии не бампнуты).

Загрузчик (ai-agent-roles-catalog.provider.ts): parseYaml через yaml@2.8.3 (safe JSON-схема, strict, maxAliasCount от billion-laughs); пути fetch → .yaml; size-cap/redirect:error/timeout/SSRF-гард без изменений. Валидатор check.mjs, README, .env.example обновлены. Тесты: YAML-парсинг, round-trip блок-скаляра, malformed→BadGateway, дубль-ключ→BadGateway, size-cap/path-traversal на .yaml. tsc + 124 jest + check.mjs + build зелёные.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

🤖 Generated with Claude Code

Closes #229. Каталог ролей переведён на **YAML**: `index.yaml` + `bundles/<id>/<lang>.yaml`, `instructions` — литеральный блок-скаляр (`|-`) → построчные диффы, редактируется как обычный текст. Конвертация программная с `deepStrictEqual` round-trip против старого JSON (контент байт-в-байт, версии не бампнуты). Загрузчик (`ai-agent-roles-catalog.provider.ts`): `parseYaml` через `yaml@2.8.3` (safe JSON-схема, `strict`, `maxAliasCount` от billion-laughs); пути fetch → `.yaml`; size-cap/redirect:error/timeout/SSRF-гард без изменений. Валидатор `check.mjs`, README, `.env.example` обновлены. Тесты: YAML-парсинг, round-trip блок-скаляра, malformed→BadGateway, дубль-ключ→BadGateway, size-cap/path-traversal на `.yaml`. tsc + 124 jest + check.mjs + build зелёные. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Ghost added 1 commit 2026-06-27 06:37:00 +03:00

refactor(agent-roles-catalog): store catalog as YAML with block-scalar instructions (#229 ) 0ec954b382

The agent-roles catalog content files move from JSON to YAML so each role's long
`instructions` system prompt is stored as a literal block scalar (`|-`): editing
one sentence now produces a line-by-line diff and the prompt is editable as plain
multi-line text instead of a single escaped JSON string.

Data:
- `index.json` -> `index.yaml`, `bundles/<id>/<lang>.json` -> `<lang>.yaml`
  (old `.json` deleted). Converted programmatically via the `yaml` library with
  `lineWidth: 0`; round-trip verified deepEqual against the old JSON, so the
  resolved role content is byte-for-byte identical and no `version` is bumped
  (content-hash lock unchanged).

Server (`AiAgentRolesCatalogProvider`):
- parse with `yaml`'s safe default (JSON-compatible) schema instead of
  `JSON.parse` — `strict: true` (rejects unknown tags / duplicate keys) and
  `maxAliasCount: 100` (billion-laughs guard); no custom `!!` tags / no code
  execution. Fetched paths become `index.yaml` / `<lang>.yaml`. The streaming
  1 MB size cap, `redirect: 'error'`, 10s timeout and `^[a-z0-9-]+$`
  path-traversal/SSRF guard are unchanged; the hand-written type guards are
  untouched (`instructions` is still a string after parsing).
- add `yaml` as a direct server dependency (already in the lockfile as a
  transitive dep).

Catalog tooling:
- `scripts/check.mjs` parses the catalog as YAML (lockfile stays JSON); pin
  `yaml` as a devDependency of the catalog package.

Tests:
- provider spec fixtures serialized with `yaml`; new tests for the block-scalar
  `instructions` round-trip (exact multi-line string), malformed YAML and
  strict duplicate-key rejection -> BadGateway; size-cap and path-traversal
  cases retargeted to the `.yaml` paths.

Docs: README, `.env.example`, `catalog-types.ts` comments and CHANGELOG updated
to the YAML layout. `AI_AGENT_ROLES_CATALOG_URL` base-URL contract unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

vvzvlad commented

2026-06-28 04:02:43 +03:00

Code review — PR #231: миграция каталога agent-roles JSON → YAML

Вердикт: Request changes (нужны правки).

Сама конвертация сделана аккуратно: YAML побайтово эквивалентен JSON на точке ветвления (deepStrictEqual 5/5, node scripts/check.mjs зелёный), парсинг безопасен (safe-схема yaml, без выполнения кода; maxAliasCount гасит billion-laughs независимо от 1 MB cap; SSRF/redirect:error/timeout/size-cap/SEGMENT_RE нетронуты; секретов в диффе нет). Блокируют merge две вещи: ветка отстала от develop и при слиянии теряется обновление роли fact-checker (v3), а также остался один устаревший комментарий «untrusted JSON».

Must fix before merge

[regressions] Перебазировать ветку на актуальный develop и перенести в YAML контент fact-checker v3, затем обновить content-hashes.json. — agent-roles-catalog/index.yaml, bundles/editorial/{en,ru}.yaml, scripts/content-hashes.json
Ветка отрезана от merge-base 904f7b4 и конвертирует старый (v2) контент fact-checker. Тем временем в develop уже влит коммит 89edddc5 («fact-checker flags errors instead of confirming facts»): index.json поднял версию 2→3, переписаны description+instructions в editorial/{en,ru}.json, обновлён content-hashes.json. Этот PR удаляет те .json, поэтому 3-way merge конфликтует modify/delete именно на этих файлах (отсюда mergeable:false). Если разрешить конфликт «просто выбросить .json», то index.yaml останется с fact-checker version: 2 и старым текстом — улучшение из develop пропадёт. При этом content-hashes.json (его PR не трогает) на пост-merge дереве ждёт v3, поэтому check.mjs упадёт (content changed but version was not bumped (still 3)) — молча в прод стухшее не уедет, но подсказка самого чека («bump version, then --update-hashes») при наивном следовании зафиксирует старый v2-хеш под новой версией и узаконит регресс зелёным чеком. Правильный путь: rebase на develop, регенерировать YAML из v3-JSON (в index.yaml fact-checker version: 3), затем node scripts/check.mjs --update-hashes и закоммитить обновлённый content-hashes.json в этом же PR.
[documentation] Заменить устаревший комментарий «untrusted JSON» → «untrusted YAML». — apps/server/src/core/ai-chat/roles/ai-agent-roles.service.ts:190
Строка // Catalog (admin-only). The catalog is curated, untrusted JSON fetched + — единственная оставшаяся в server-коде ссылка на старый формат (проверено grep'ом по всему репозиторию; catalog-types.ts и provider.ts PR уже поправил на YAML). Каталог теперь YAML, комментарий противоречит коду. Поменять JSON → YAML.
[suggestion][documentation] Уточнить docstring parseYaml про strict: true. — apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.ts:87-90
Комментарий утверждает, что strict: true «rejects unknown custom tags». По факту в yaml@2.8.3 неизвестный тег (!!js/function …) не бросает — выдаёт warning и резолвит узел в обычный скаляр (а дальше его отсекает hand-written type guard). Реальная гарантия безопасности (core-схема не конструирует не-JSON типы и не исполняет код; type guard режет лишнее) сохраняется, и часть про duplicate keys верна — переформулировать только пункт про custom tags, чтобы не вводить в заблуждение про security-critical парсинг.
[suggestion][conventions] Задокументировать, что check.mjs берёт yaml из корневого node_modules (через shamefully-hoist), а не из своего пакета. — agent-roles-catalog/scripts/check.mjs:11-13, agent-roles-catalog/package.json:7-9
agent-roles-catalog/ вне pnpm-workspace (pnpm-workspace.yaml = только apps/*, packages/*) и не имеет своего node_modules. import "yaml" резолвится вверх по дереву в корневой node_modules/yaml — но он там есть только потому, что apps/server объявляет yaml, а .npmrc (shamefully-hoist=true) поднимает его в корень. Добавленный блок devDependencies.yaml в agent-roles-catalog/package.json при этом инертен (pnpm его не ставит). В чистом checkout без корневого pnpm install валидатор упал бы ERR_MODULE_NOT_FOUND; blast-radius низкий (в CI/Docker check.mjs не вызывается). Добавить в README (секция «Validating») строку, что нужен предварительный pnpm install в корне; либо объявить yaml в корневом package.json, чтобы не зависеть от хойстинга.

Test coverage

Покрыто осмысленными ассертами: happy-path YAML (index + bundle), malformed-YAML→BadGateway, duplicate-key (strict)→BadGateway, malformed-shape→BadGateway, round-trip блок-скаляра instructions (точное многострочное сравнение), size-cap / path-traversal на .yaml, fallback response.text(). Пробелы по новой логике этого PR:

[warning] Нет теста на новый guard maxAliasCount (alias-bomb). — …/ai-agent-roles-catalog.provider.ts:95
maxAliasCount: 100 добавлен как защита от billion-laughs (об этом прямо сказано в docstring), но ни один тест не скармливает парсеру alias-тяжёлый документ. Добавить тест: тело YAML с вложенными алиасами свыше лимита → fetchIndex() reject BadGatewayException (заодно проверит, что throw мапится, а не утекает как 500).
[warning] Нет теста на YAML-коэрцию неэкранированного скаляра в не-строку. — …/ai-agent-roles-catalog.provider.spec.ts:299-311
Переход с JSON на YAML рождает новый failure-mode, которого у JSON не было: slug: 12 / name: no (без кавычек) YAML приводит к числу/булеву, а защищает только type guard. Единственный malformed-shape тест использует явно не-строковое значение (schemaVersion: 'x') — это и JSON мог бы. Добавить кейс с сырой YAML-строкой (не через stringifyYaml, иначе значение перекавычится), где строковое поле задано неэкранированным скаляром, и проверить reject BadGatewayException.
[suggestion] Усилить ассерт на пустое тело→null→reject. — …/ai-agent-roles-catalog.provider.spec.ts:159-168
Смена парсера меняет поведение: JSON.parse('') бросал, parseYamlDoc('') возвращает null (далее отсекается guard'ом). Сейчас это лишь побочно задето в тесте про redirect через rejects.toBeDefined() — пройдёт даже если пустое тело утечёт как 500. Добавить отдельный тест: пустое/whitespace тело → reject BadGatewayException (/malformed/i).

Architecture & design (forward-looking, non-blocking)

None. Раздвоение валидации (runtime type-guards в provider.ts vs кросс-файловые проверки + version-gate в check.mjs) — преджестирующая, когерентная архитектура в раздельных пакетах; этот PR её не ухудшает. Зависимость yaml оправдана (уже была транзитивной — promote без новой supply-chain поверхности), мёртвого кода/лишних тестов не внесено.

_{🤖 Multi-aspect review (security · stability · conventions · documentation · regressions · test-coverage · simplification · architecture). Generated with Claude Code.}

## Code review — PR #231: миграция каталога agent-roles JSON → YAML **Вердикт: Request changes (нужны правки).** Сама конвертация сделана аккуратно: YAML **побайтово эквивалентен** JSON на точке ветвления (`deepStrictEqual` 5/5, `node scripts/check.mjs` зелёный), парсинг безопасен (safe-схема `yaml`, без выполнения кода; `maxAliasCount` гасит billion-laughs независимо от 1 MB cap; SSRF/`redirect:error`/timeout/size-cap/`SEGMENT_RE` нетронуты; секретов в диффе нет). Блокируют merge две вещи: ветка **отстала от `develop`** и при слиянии теряется обновление роли `fact-checker` (v3), а также остался один устаревший комментарий «untrusted JSON». ### Must fix before merge - **[regressions] Перебазировать ветку на актуальный `develop` и перенести в YAML контент `fact-checker` v3, затем обновить `content-hashes.json`.** — `agent-roles-catalog/index.yaml`, `bundles/editorial/{en,ru}.yaml`, `scripts/content-hashes.json` Ветка отрезана от merge-base `904f7b4` и конвертирует **старый (v2)** контент `fact-checker`. Тем временем в `develop` уже влит коммит `89edddc5` («fact-checker flags errors instead of confirming facts»): `index.json` поднял версию 2→3, переписаны `description`+`instructions` в `editorial/{en,ru}.json`, обновлён `content-hashes.json`. Этот PR **удаляет** те `.json`, поэтому 3-way merge конфликтует modify/delete именно на этих файлах (отсюда `mergeable:false`). Если разрешить конфликт «просто выбросить .json», то `index.yaml` останется с `fact-checker version: 2` и старым текстом — улучшение из `develop` пропадёт. При этом `content-hashes.json` (его PR не трогает) на пост-merge дереве ждёт v3, поэтому `check.mjs` **упадёт** (`content changed but version was not bumped (still 3)`) — молча в прод стухшее не уедет, но подсказка самого чека («bump version, then --update-hashes») при наивном следовании зафиксирует старый v2-хеш под новой версией и **узаконит регресс зелёным чеком**. Правильный путь: rebase на `develop`, регенерировать YAML из v3-JSON (в `index.yaml` `fact-checker version: 3`), затем `node scripts/check.mjs --update-hashes` и закоммитить обновлённый `content-hashes.json` в этом же PR. - **[documentation] Заменить устаревший комментарий «untrusted JSON» → «untrusted YAML».** — `apps/server/src/core/ai-chat/roles/ai-agent-roles.service.ts:190` Строка `// Catalog (admin-only). The catalog is curated, untrusted JSON fetched +` — единственная оставшаяся в server-коде ссылка на старый формат (проверено grep'ом по всему репозиторию; `catalog-types.ts` и `provider.ts` PR уже поправил на YAML). Каталог теперь YAML, комментарий противоречит коду. Поменять `JSON` → `YAML`. - **[suggestion][documentation] Уточнить docstring `parseYaml` про `strict: true`.** — `apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.ts:87-90` Комментарий утверждает, что `strict: true` «rejects unknown custom tags». По факту в `yaml@2.8.3` неизвестный тег (`!!js/function …`) **не бросает** — выдаёт warning и резолвит узел в обычный скаляр (а дальше его отсекает hand-written type guard). Реальная гарантия безопасности (core-схема не конструирует не-JSON типы и не исполняет код; type guard режет лишнее) сохраняется, и часть про duplicate keys верна — переформулировать только пункт про custom tags, чтобы не вводить в заблуждение про security-critical парсинг. - **[suggestion][conventions] Задокументировать, что `check.mjs` берёт `yaml` из корневого `node_modules` (через `shamefully-hoist`), а не из своего пакета.** — `agent-roles-catalog/scripts/check.mjs:11-13`, `agent-roles-catalog/package.json:7-9` `agent-roles-catalog/` вне pnpm-workspace (`pnpm-workspace.yaml` = только `apps/*`, `packages/*`) и не имеет своего `node_modules`. `import "yaml"` резолвится вверх по дереву в корневой `node_modules/yaml` — но он там есть только потому, что `apps/server` объявляет `yaml`, а `.npmrc` (`shamefully-hoist=true`) поднимает его в корень. Добавленный блок `devDependencies.yaml` в `agent-roles-catalog/package.json` при этом инертен (pnpm его не ставит). В чистом checkout без корневого `pnpm install` валидатор упал бы `ERR_MODULE_NOT_FOUND`; blast-radius низкий (в CI/Docker `check.mjs` не вызывается). Добавить в README (секция «Validating») строку, что нужен предварительный `pnpm install` в корне; либо объявить `yaml` в корневом `package.json`, чтобы не зависеть от хойстинга. ### Test coverage Покрыто осмысленными ассертами: happy-path YAML (index + bundle), malformed-YAML→BadGateway, duplicate-key (`strict`)→BadGateway, malformed-shape→BadGateway, round-trip блок-скаляра `instructions` (точное многострочное сравнение), size-cap / path-traversal на `.yaml`, fallback `response.text()`. Пробелы по **новой** логике этого PR: - **[warning] Нет теста на новый guard `maxAliasCount` (alias-bomb).** — `…/ai-agent-roles-catalog.provider.ts:95` `maxAliasCount: 100` добавлен как защита от billion-laughs (об этом прямо сказано в docstring), но ни один тест не скармливает парсеру alias-тяжёлый документ. Добавить тест: тело YAML с вложенными алиасами свыше лимита → `fetchIndex()` reject `BadGatewayException` (заодно проверит, что throw мапится, а не утекает как 500). - **[warning] Нет теста на YAML-коэрцию неэкранированного скаляра в не-строку.** — `…/ai-agent-roles-catalog.provider.spec.ts:299-311` Переход с JSON на YAML рождает новый failure-mode, которого у JSON не было: `slug: 12` / `name: no` (без кавычек) YAML приводит к числу/булеву, а защищает только type guard. Единственный malformed-shape тест использует **явно** не-строковое значение (`schemaVersion: 'x'`) — это и JSON мог бы. Добавить кейс с сырой YAML-строкой (не через `stringifyYaml`, иначе значение перекавычится), где строковое поле задано неэкранированным скаляром, и проверить reject `BadGatewayException`. - **[suggestion] Усилить ассерт на пустое тело→null→reject.** — `…/ai-agent-roles-catalog.provider.spec.ts:159-168` Смена парсера меняет поведение: `JSON.parse('')` бросал, `parseYamlDoc('')` возвращает `null` (далее отсекается guard'ом). Сейчас это лишь побочно задето в тесте про redirect через `rejects.toBeDefined()` — пройдёт даже если пустое тело утечёт как 500. Добавить отдельный тест: пустое/whitespace тело → reject `BadGatewayException` (`/malformed/i`). ### Architecture & design (forward-looking, non-blocking) None. Раздвоение валидации (runtime type-guards в `provider.ts` vs кросс-файловые проверки + version-gate в `check.mjs`) — преджестирующая, когерентная архитектура в раздельных пакетах; этот PR её не ухудшает. Зависимость `yaml` оправдана (уже была транзитивной — promote без новой supply-chain поверхности), мёртвого кода/лишних тестов не внесено. <sub>🤖 Multi-aspect review (security · stability · conventions · documentation · regressions · test-coverage · simplification · architecture). Generated with [Claude Code](https://claude.com/claude-code).</sub>

Ghost force-pushed feat/229-catalog-yaml from 0ec954b382 to 38a863e5f7

2026-06-28 04:41:33 +03:00

Compare

Ghost referenced this issue from a commit

2026-06-28 04:41:33 +03:00

refactor(agent-roles-catalog): store catalog as YAML with block-scalar instructions (#229)

Ghost added the review/changes-requested label 2026-06-28 22:29:48 +03:00

Ghost referenced this issue from a commit

2026-06-28 23:45:12 +03:00

test(agent-roles-catalog): pin the real shipped YAML files (#231 F1)

Ghost referenced this issue from a commit

2026-06-28 23:45:12 +03:00

docs(changelog): drop duplicate "### Changed" header (#231 F2)

Ghost added 2 commits 2026-06-28 23:45:12 +03:00

test(agent-roles-catalog): pin the real shipped YAML files (#231 F1) 997e4395c6

Provider tests only exercised synthetic stringifyYaml fixtures, so a
hand-conversion error in one of the 6 real catalog files (index.yaml,
bundles/{editorial,research}/{en,ru}.yaml) — a stray quote/colon in a
description, a broken emoji/arrow, a block-scalar indent slip that
silently changes or drops instructions — was caught by no automated
test. scripts/check.mjs is the only other guard and is wired into no
CI/turbo/husky step.

Add a real-files test block that reads each shipped file off disk,
parses it with the SAME options the provider uses
(strict: true, maxAliasCount: 100), and validates it through the
provider's own exported type guards (isCatalogIndex / isCatalogBundleFile
/ isCatalogRole). It is driven from the real index so new bundles/langs
are auto-covered, asserts the editorial bundle still ships fact-checker,
and requires every declared role to be present with non-empty
instructions/name in each language file.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

docs(changelog): drop duplicate "### Changed" header (#231 F2) 2c1fe98404

The YAML-migration entry (#229) added a second "### Changed" header in
the same [Unreleased] group that already had one (#216), rendering as two
Changed sections and violating Keep a Changelog. Remove the duplicate
header so the #229 bullet falls under the existing Changed section.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Ghost added review/needs and removed review/changes-requested labels 2026-06-28 23:45:43 +03:00

Ghost added 1 commit 2026-06-29 00:00:05 +03:00

test(catalog): tighten + isolate real shipped catalog-file checks 82af0c5291

Apply review suggestions to the real-files block in
ai-agent-roles-catalog.provider.spec.ts (test-only):

1. Fix inaccurate comment: there are 5 content YAML files (index +
   four per-bundle/lang files), not 6.
2. Improve isolation: read/parse the real index lazily inside tests
   (via loadRealIndex) instead of in the describe body, so a broken
   real file fails only these catalog tests, not collection of the
   whole spec (incl. the unrelated mocked-remote provider tests).
3. Add the symmetric slug check: each language file's slug set must
   equal the declared slug set (no undeclared/extra roles), matching
   scripts/check.mjs's exact two-way correspondence.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Ghost added the status/in-progress label 2026-06-29 00:17:05 +03:00

Ghost added review/approved and removed review/needs labels 2026-06-29 00:48:32 +03:00

vvzvlad merged commit 4a72ee1681 into develop

2026-06-29 01:20:40 +03:00

vvzvlad referenced this issue from a commit

2026-06-29 01:20:42 +03:00

Merge pull request 'refactor(agent-roles-catalog): YAML catalog with block-scalar instructions (#229)' (#231) from feat/229-catalog-yaml into develop

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#231