The same tool metadata (zod schema + model-facing description) was hand-duplicated
between the standalone MCP server and the in-app AI-chat agent, so every tweak had to
land in two places and copies drifted (a materialized parity bug). The shared
transport-agnostic registry (packages/mcp/src/tool-specs.ts) already de-duplicates 14
tools; this migrates two more genuinely-identical ones — patch_node/patchNode and
insert_node/insertNode. The canonical description is a strict SUPERSET of both originals
(keeps MCP's "without resending the whole document" + table-structure/anchor guidance
AND the in-app "reversible via page history" / "exactly one of anchorNodeId or
anchorText" framing — no model-facing guidance dropped); the schema is identical (the
in-app side just gains MCP's .min(1) on ids, a safe tightening). Each transport keeps its
own execute/auth wrapper, and the in-app parseNodeArg node-arg normalization is unchanged.
The three table tools are intentionally NOT merged (a real param-name divergence:
table vs tableRef) — documented on both sides. Other per-transport divergences
(search/share/create_comment/transform/list_pages) are left separate with a short comment
explaining why (the issue asked to flag these as intentional). DocmostClientLike stays a
hand-mirror (the ESM/CJS boundary blocks a compile-time type import; a runtime drift-guard
already pins it). Also fixes a latent contract-spec bug: derive `required` from
`instanceof z.ZodOptional` (matches the emitted JSON schema) instead of `isOptional()`,
which wrongly reported z.any() fields as optional.
Partially addresses #294.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F1: pc.title (untrusted cross-user page title) was interpolated raw into the
markdown export heading. Reusing escapeAttr alone (the prompt sink's XML-attribute
sanitizer, strips < > ") is insufficient here because the sink is MARKDOWN: link
/image syntax survives, so a title like  or [phish](http://evil)
injects a remote image / clickable link into the downloaded .md disguised as a
trusted system annotation. Add markdownHeadingSafe() = escapeAttr() + backslash-
escape [ and ] (disables both [text](url) and ; a bare (url) is inert).
F2: cover the title branch — a title that collapses to empty via escapeAttr falls
to the bare heading (no ("")), and a link/image-injection title is neutralized
(non-vacuous vs the escapeAttr-only version).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The #274 page_changed marker lived only in the ephemeral system prompt, so the
diff the agent saw was invisible in the chat export/history, and the note was
too weak — the agent still overwrote the user's manual edits with a full-page
replace.
- Persist the diff the agent saw as metadata.pageChanged on the assistant row
(flushAssistant), threaded into all five flush call sites in stream(). Model
replay (rowToUiMessage/rowParts) reads only metadata.parts, so the sibling
never re-injects the note into the model context on later turns.
- Render the persisted diff as a labelled block (en/ru) before the message body
in the server-side Markdown export (chat-markdown.util.ts).
- Strengthen PAGE_CHANGED_NOTE: mandate a fresh getPage re-read and targeted
edits (editPageText/patchNode/insertNode/deleteNode) instead of a whole-page
replace, and never revert or overwrite the user's edits.
Tests: prompt, export and service specs updated; 114 pass, tsc clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
F1: escape the collaborative page title before interpolating into
<page_changed page="..."> (and the pre-existing openedPage attr) — strip
<>" and collapse whitespace, so a crafted title can't break out of the
attribute into the system prompt (cross-user injection).
F2: neutralize <page_changed>/</page_changed> occurrences inside the diff body
so a crafted line can't close the block early.
F3: remove the dead content_hash column (written every turn, never read) —
migration, repo, service hashing + crypto import, db.d.ts, spec asserts.
F4: test the best-effort catch branches (detectPageChange / snapshotOpenPage
swallow errors and don't break the turn).
F5: soften the overstated 'diff cannot smuggle instructions' comment to
defense-in-depth framing referencing the F1/F2 mitigations + safety sandwich.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The agent rebuilds context from DB each turn and didn't know the user manually
edited the open page since its last response, so it could overwrite those edits.
Add a per-turn ephemeral <page_changed> note in the system prompt (twin of
INTERRUPT_NOTE, self-clearing) carrying a unified Markdown diff of what changed
since the END of the agent's previous turn.
- New ai_chat_page_snapshots table (migration + hand-declared db.d.ts/entity
types) storing the page Markdown per (chat,page) at each turn's end.
- Pure computePageChange util (whitespace-normalized unified diff via the
existing jsdiff dep, 6KB cap + getPage hint).
- Turn start: if the open page's updatedAt moved past the snapshot, diff current
vs snapshot; non-empty -> PAGE_CHANGED_NOTE in the safety sandwich.
- Turn end: upsert the snapshot on EVERY terminal path (onFinish/onError/onAbort,
once) so the agent's own edits are excluded by construction even on aborted
turns.
All best-effort (never breaks/latency-regresses a turn); fast path when updatedAt
is unchanged. Server-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F11: PRE_SEED_TTL_SECONDS 45->120 (= client REINDEX_POLL_CAP_MS). At concurrency
1 a queued reindex can wait past the old 45s; if the pre-seed expired while
pending, getMasked fell back to the COUNT and reported done, so the client
stopped polling and missed the climb. Tie the pre-seed TTL to the client cap.
F12: extend the lockstep integration spec — insertPage takes content; a
text_content=null + text-node-content page is IN and a math-only page is OUT,
pinning the structural "type":"text" clause (and the jsonb space-after-colon).
F13: list all three embeddable clauses in the reindex JSDoc/inline comments.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
guardedFetch blocks loopback/private/link-local/metadata IPs and never calls
fetch; decryptHeaders fails open (returns undefined, warns once, no blob leak).
yjs.util setYjsMark/removeYjsMarkByAttribute/updateYjsMarkAttribute on real
Y.Docs. SHARED_TOOL_SPECS<->in-app parity (name/desc/input-schema; a dropped or
renamed wiring fails). Replace the tautological storage.service spec with
driver-delegation checks across every public method.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
The blob-sandbox feature (#243/#250) made AiChatToolsService.forUser()
eagerly call this.sandboxStore.asSink() while wiring the stash tool, but
the spec still passed an empty {} as the sandboxStore constructor arg.
That object has no asSink method, so all 19 tests in the suite failed in
CI with 'TypeError: this.sandboxStore.asSink is not a function'.
Replace the stale {} mock at all 4 constructor sites with a no-op sink
exposing asSink() -> { put, has, evict } (jest.fn()). These tests never
execute the stash tool, so a no-op sink is sufficient for forUser() to
wire successfully. Test-only change; production code is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
F1: clear the "Reindex now" spinner once the poll cap fires. Gate the
reindexing part of the button's loading state on the active poll window
(reindexDeadline !== null) so a run that outlives the 120s cap no longer
leaves the button stuck-disabled with a stale `reindexing: true`; the
admin can restart.
F2: rewrite reindexWorkspace JSDoc to describe the EMBEDDABLE page set
(text OR existing embeddings), matching getEmbeddablePageIds /
countEmbeddablePages instead of the old "every non-deleted page".
F3: extract the shared embeddable-content predicate into a private
PageRepo.embeddablePredicate helper, called by both countEmbeddablePages
and getEmbeddablePageIds, removing the verbatim duplication. Behavior is
identical (lockstep int-spec stays green).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Security (must-fix):
- sandbox.controller: the anonymous GET /api/sb/:id response now sets
X-Content-Type-Options: nosniff, a restrictive CSP, and Content-Disposition=
attachment for any mime outside a raster-image allowlist (png/jpeg/gif/webp/
avif). entry.mime is attacker-controlled, so an evil.svg/evil.html could
otherwise execute script inline on the Docmost origin (stored XSS). Mirrors
the public attachment route's hardening.
Stability:
- client.stashPage: reconcile mirrors AFTER the final document put, not only
before it. The doc blob is the newest entry and FIFO eviction drops the
oldest = this stash's own images, so the stored doc could reference an
evicted blob (consumer 404) and over-report images.mirrored. A bounded loop
now reverts doc-put-evicted mirrors, drops the stale doc blob, and re-puts
until stable. Regenerated packages/mcp/build/.
- sandbox.controller: emit Cache-Control on the 304 branch too (ttlSeconds is
computed before the conditional check).
Docs:
- Bump the MCP tool count 39 -> 40 across all READMEs and AGENTS.md (the
registry now exposes exactly 40 tools).
Refactor:
- SandboxStore.asSink() centralizes the {put,has,evict} sink + uri<->id
mapping; the embedded-MCP and in-app agent-tools wiring sites share it.
Tests:
- security headers (inline vs attachment, nosniff, CSP), 304 Cache-Control,
putAndLink URL form, has()/remove(), asSink() round-trip, getSandboxPublicUrl
(trailing-slash trim + APP_URL fallback), and a stash test where the doc put
itself evicts a mirrored image.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Security:
- stash_page: reject path-traversal / percent-encoded srcs before the authed
loopback fetch (resolveInternalFilePath), closing an SSRF/exfiltration hole
where a crafted node.attrs.src could read an arbitrary internal GET endpoint
into the anonymous sandbox.
Stability:
- stash_page: revert + recount mirrors FIFO-evicted by a later put in the same
stash (no dangling sandbox refs, honest images.mirrored/failed); free image
blobs if the final document put throws.
- Reject/clamp non-positive SANDBOX_TTL_MS to the 1h default (warn once).
- Log mirror failures unconditionally (console.warn, no blob bodies).
Cleanup / architecture:
- Remove dead expiresAt from SandboxPutResult.
- Centralize the /api/sb route in SANDBOX_ROUTE_SEGMENT/SANDBOX_API_PATH and
move URL composition into SandboxStore.putAndLink; drop the duplicated sink
closures and the now-unused EnvironmentService injection from McpService and
AiChatToolsService.
- Un-export isInternalFileUrl; document the process-local (instance-bound)
sandbox limitation in the tool description and .env.example.
Docs/tests:
- README/README.ru: 38 -> 39 tools + stash_page entry.
- Add traversal/normalize/recursion unit tests, stash self-eviction +
doc-put-throw + empty/octet-stream mock tests, controller If-None-Match
(wildcard/weak/list) + Cache-Control tests, and SANDBOX_TTL_MS validation
tests. Regenerate packages/mcp/build.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an ephemeral, process-local blob store so the in-app agent (and the
embedded MCP) can hand a large page document and its images to an external
consumer WITHOUT routing the bytes through the model context or Docmost auth.
- SandboxStore (@Injectable singleton): Map<uuid,{buf,mime,sha256,expiresAt}>
in RAM only. put() picks a per-blob cap by mime (image vs doc), enforces a
total-bytes RAM guard with oldest-first eviction, and stamps a TTL; get()
lazily expires. sha256 computed at put() doubles as the strong ETag. An
unref'd sweep interval clears expired entries and is cleared on destroy.
- GET /api/sb/:uuid anonymous controller: serves raw bytes with Content-Type,
Content-Length and ETag=sha256; 404 on missing/expired/non-UUID (anti-
traversal), 304 on a matching If-None-Match. No tokens, no 401 — the
capability is the unguessable UUID + short TTL + TLS. Auth-exempt the same
way as /api/files/public (no JwtAuthGuard) plus an /api/sb entry in main.ts's
workspace-resolution preHandler so a remote consumer with no workspace host
is not rejected.
- stash_page tool in both layers (MCP resource_link + in-app {uri,size,sha256,
images}). client.stashPage serializes the get_page_json shape, mirrors every
INTERNAL file/image src (type-agnostic, covers drawio/excalidraw/video/file)
into the sandbox under Docmost auth and rewrites src to the sandbox URL;
external http(s) srcs are left untouched; dedup by src; a failed image fetch
is counted, never aborts the doc.
- SANDBOX_PUBLIC_URL / SANDBOX_TTL_MS / SANDBOX_MAX_BYTES /
SANDBOX_MAX_IMAGE_BYTES / SANDBOX_MAX_TOTAL_BYTES wired through the
environment service + validation + .env.example.
- SandboxModule (@Global) provides the shared store to the controller,
McpService and AiChatToolsService (same instance for put and get).
Tests: SandboxStore (round-trip, sha256, TTL lazy + sweep, caps, eviction),
SandboxController (200+ETag+CT+CL, 404 missing/expired/non-UUID, 304), and a
mock-HTTP stashPage test (mirror+rewrite internal, keep external, dedup, failed
image counted, returns only a link). Interoperates with the vvzvlad/habr-mcp
consumer's anonymous-GET + sha256-ETag + resource_link contract.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Issue #193's tool-half has two open items. The shared, zod-agnostic tool-spec
registry (SHARED_TOOL_SPECS) for the identical tools is already merged
(f3fa15e7) and consumed by both layers, so that subset is done. The remaining
items are: (a) deriving the layer-3 hand-mirror `DocmostClientLike` from the
real client type, and (b) folding more tools into the registry. Both were
deferred as risky, and that deferral still holds (verified, see below) — so
this change ships the safest concrete increment instead of forcing the risk.
What this adds (behaviour-neutral, test-only + a doc comment):
- packages/mcp/test/unit/client-host-contract.test.mjs: pins the layer-3
contract from the ESM side, where the real DocmostClient is importable. It
asserts every method the in-app `DocmostClientLike` mirror declares exists as
a function on a real DocmostClient instance (constructor is side-effect-free).
A rename/removal in client.ts now fails this test instead of silently shipping
a runtime "x is not a function" into an agent tool call. Negative-case
verified (a bogus method name is detected).
- docmost-client.loader.ts: replaces the vague mirror comment with a pointer to
the guard test and a concrete, empirically-grounded staged plan for the full
type-derivation. Verified blockers kept it deferred: @docmost/mcp emits no
.d.ts (no `declaration`, no `types` export) and the server has no path mapping
for it, so there is no type to import today; and the real methods' inferred
CONCRETE return types conflict with the in-app adapter's loose
Record<string,unknown> + `as`-cast result handling (deriving the exact type
breaks the build / forces pervasive double-casts and full-surface test stubs).
Out of scope (noted in the issue): the PM<->Markdown converter unification.
Verified: server tsc clean; mcp tsc clean; mcp tests 369 pass (367 + 2 new);
ai-chat tools specs 51 pass. No behaviour change; committed mcp build untouched
(no mcp src changed).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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 (the only `version` bump is
fact-checker v2->3, carried over from develop during the rebase; see below).
Server (`AiAgentRolesCatalogProvider`):
- parse with `yaml`'s safe default (JSON-compatible) schema instead of
`JSON.parse` — `strict: true` (rejects 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.
Rebase onto develop + review (PR #231, comment 2509):
- semantic conflict: develop's 89edddc5 bumped fact-checker v2->3 (flags errors
instead of confirming facts) in the now-deleted `.json`. Resolved the
modify/delete by taking the deletion and porting develop's v3 `description` +
`instructions` (en + ru) into the YAML and setting `version: 3` in index.yaml.
Verified by `node scripts/check.mjs` going green against develop's unchanged
content-hash lock (the ported YAML hashes byte-identically to the v3 JSON).
- doc fix: ai-agent-roles.service.ts catalog comment "untrusted JSON" -> YAML.
- doc fix: parseYaml docstring no longer claims `strict: true` rejects unknown
custom tags (yaml@2.8.x warns + resolves to a plain scalar, then the type
guard rejects it); the duplicate-key claim is kept.
- doc: note in check.mjs that `yaml` resolves from the repo-ROOT node_modules
(via shamefully-hoist), not the catalog package's own pinned devDependency.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review fixes for the reindex-progress counter (#242):
1. Denominator jump (478 -> 500 -> 478): reindexWorkspace iterated
getIdsByWorkspace() (ALL non-deleted pages) but the seed/status use
countEmbeddablePages (text OR existing-embedding), so the live total exceeded
the steady-state total whenever empty/text-less pages existed. Add
PageRepo.getEmbeddablePageIds() that selects the IDs of the EXACT same set
countEmbeddablePages counts (deletedAt IS NULL AND (text_content matches a
non-whitespace char OR an EXISTS non-deleted pageEmbeddings row)), and have
reindexWorkspace iterate THAT set with total = its length. Iteration set and
count source change together, so done reaches exactly total == the
steady-state denominator. Dropping text-less pages is correct (reindexPage
no-ops on them; a page that lost its text but still has stale embeddings is in
the set via the EXISTS clause and still gets its stale rows cleared). Removed
the contradictory "worker overwrites with the real page count" / "denominator
matches" comment.
2. Mid-run re-trigger reset: reindex() unconditionally re-seeded done=0 before an
enqueue that de-dupes a running job, so a second click/admin/tab reset the
visible counter while the worker kept incrementing. Now seed only when
get(workspaceId) === null; the worker's own start() remains the single
authoritative reset.
3. TTL: documented that it is intentionally tied to write progress
(start/increment) and never refreshed on get(), so a dead worker's record
can't be kept alive forever by client polling.
Tests: new embedding-reindex-progress.service.spec.ts (fake ioredis: hash ->
ReindexProgress, malformed/missing/non-numeric -> null, non-finite startedAt ->
0, hgetall throws -> null, start/increment issue hset/hincrby+expire and swallow
Redis errors); reindex() seed order + no-reseed-when-active guard; getMasked
live test now uses progress.total=500 vs DB 478 to pin the progress branch;
indexer specs updated to mock getEmbeddablePageIds.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "Indexed X of Y pages" counter stayed stuck at "478 of 478" during a
manual "Reindex now" run instead of resetting to 0 and climbing. The status
reports indexedPages = countIndexedPages (DISTINCT pages with >=1 embedding
row), but reindex hard-replaces each page in its OWN small transaction, so
nearly all pages always have rows -> the count never drops.
Add a per-workspace live reindex-progress record in Redis (reusing the
existing global ioredis client via RedisService, no new Redis config):
- EmbeddingReindexProgressService: start/increment/clear/get over a Redis hash
with a 1h TTL self-clean; all best-effort/cosmetic so a Redis failure degrades
to the existing DB-count behavior.
- AiSettingsService.reindex seeds {total, done:0, startedAt} at enqueue time so
the very first poll already reports done=0.
- EmbeddingIndexerService.reindexWorkspace overwrites total with the real page
count at start, increments done per processed page (success or handled
failure), and clears the record in a finally (covers success, fatal abort,
and the unconfigured early-return) so a failed run never sticks.
- AiSettingsService.getMasked returns the live run numbers when a progress
record is active (plus an optional reindexing flag), else falls back to
countIndexedPages/countEmbeddablePages.
Per-page edits (reindexPage) never touch the workspace progress record, and no
mass up-front delete is introduced (search availability preserved).
Tests: indexer sets/increments/clears progress (incl. fatal abort and
unconfigured early-return); status reports run progress when active and falls
back when not.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Additive test coverage across server, editor-ext, client and mcp.
#192 — AiChatService.stream integration (Section 3, against real Postgres):
- new apps/server/test/integration/ai-chat-stream.int-spec.ts drives the real
streamText through a seeded ai/test MockLanguageModelV3 and a real Node
ServerResponse, covering: onError persists an assistant error record
(status 'error' + partial answer + provider cause in metadata); external MCP
client closed exactly once on BOTH onFinish and onError; anti-tamper —
history is rebuilt from the DB transcript, not from body.messages.
#206 — red-team findings (most already fixed+tested in #212):
- mdrt-2 (UNFIXED, data loss): turndown.dataloss.test.ts documents that
pageBreak / transclusionReference / mention are silently dropped on Markdown
export (characterization + it.fails for the desired survive-export contract).
- persist-6 (UNFIXED, data loss): persistence-store.spec.ts adds an it.failing
documenting that a momentarily-empty live doc overwrites non-empty content
(left unfixed — a store-side empty-guard is a behaviour change).
#204 — test-strategy plan, highest-priority subset:
- Phase 1: mcp-clients.lease.spec.ts covers the external MCP client
lease/refcount/eviction lifecycle (leak / premature-close / double-close).
- Phase 2 data-integrity pure functions: editor-ext table-utils
(transpose/moveRow/convert round-trip) and math tokenizer false-positive
guard; client emoji-menu (+ it.fails for the unguarded localStorage
JSON.parse bug), sort-cells, normalizeTableColumnWidths; mcp htmlEmbed/
pageBreak markdown data-loss + footnote-diff; server export
getInternalLinkPageName extensionless-path bug — FIXED (small/clear) + tested.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The agent-roles catalog source is no longer hardcoded in app code and no
longer supports a local filesystem directory. The provider now fetches only
from an http(s):// base URL read from AI_AGENT_ROLES_CATALOG_URL; an empty or
non-http value yields a 502 (catalog unavailable). The default URL is baked
into the Docker image at build time and set per branch in CI.
- provider: drop readLocal + node:fs/node:path; readRelative requires http(s)
and 502s otherwise; remote fetch/streaming-cap/SSRF guards unchanged.
- environment.service: keep AI_AGENT_ROLES_CATALOG_URL (default ''); comment
updated to reflect build-time injection, remote-only.
- Dockerfile: add ARG+ENV AI_AGENT_ROLES_CATALOG_URL in the installer stage.
- CI: develop.yml builds with the develop raw URL; release.yml (both build
steps) with the main raw URL.
- tests: replace local-fixture tests with remote-mock happy/malformed bundle
tests and a non-http => 502 case; path-traversal block uses an https source.
- docs: update .env.example, CHANGELOG (#222), agent-roles-catalog/README.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ITEM 1: cover useImportAiRolesFromCatalogMutation onSuccess notifications.
Add import-from-catalog-message.test.tsx (twin of update-from-catalog-message)
asserting the always-shown summary (errors:[]) and the additional red
"Failed to import N role(s)" notification when result.errors is non-empty.
ITEM 2: pass redirect:'error' to the remote catalog fetch in fetchRemote so a
compromised-but-trusted upstream cannot 3xx the fetch into the internal network
(redirect-SSRF). Add provider specs asserting the option is passed and that a
redirect rejection maps to BadGatewayException.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MUST-FIX
- isSourceUniqueViolation read the wrong error field: kysely-postgres-js
(postgres@3.4.8) puts the violated constraint on `constraint_name`, not
node-postgres' `.constraint`, so a concurrent same-slug+language import's
23505 was never recognized as a source-collision and surfaced a false
"name already exists" error. Now read `constraint_name` (with `.constraint`
as a fallback for other drivers). Fix the faked test fixture (it built the
error with the same wrong `.constraint` field, masking the bug): it now
uses `constraint_name`, so the test genuinely exercises the skip path and
FAILS against the unfixed code.
- Extract the catalog modal's role-state computation into a pure
`catalogRoleInstallState(role, workspaceRoles, language)` helper (mirrors
role-launch.ts) and cover it with vitest: import / installed / update /
same-slug-different-language.
SUGGESTIONS
- Restore IAiRoleUpdateFromCatalogResult as a discriminated union mirroring
the server; narrow the consumer via `"reason" in result` (the boolean
discriminant does not narrow under strictNullChecks:false).
- README: add a "How it's served" section documenting AI_AGENT_ROLES_CATALOG_URL
(remote http(s) base / local path / empty => in-repo folder).
- check.mjs: drop the redundant `const key = slug` alias.
- Cover the reason->message mapping in useUpdateAiRoleFromCatalogMutation
(4 branches) via renderHook with a mocked service.
- Cover importFromCatalog "bundle not in index" => BadGateway.
- Cover updateFromCatalog "slug in index but missing in bundle file" =>
not-in-catalog.
ARCHITECTURE
- Extract the shared catalog read prefix: a private `loadBundleById`
(fetchIndex -> meta -> fetchBundle -> versionMap) reused by getCatalogBundle
and importFromCatalog, and a `catalogRoleContentFields` mapper shared by the
import insert and update patch. The three orchestrations and their distinct
write paths stay separate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Item 1 (concurrency-safe import): add a partial UNIQUE index on
(workspace_id, source->>'slug', source->>'language') WHERE source IS NOT NULL
AND deleted_at IS NULL, so two concurrent imports of the same bundle can no
longer create duplicate roles for one catalog slug+language. The in-memory
installedKeys snapshot cannot see a sibling request's writes; the index is the
backstop. importFromCatalog now catches the 23505 from THIS index (keyed off
the constraint name) and treats it as "already installed" -> skip, batch
continues. A 23505 from the name-uniqueness index keeps its existing friendly
per-role error behavior (distinguished by constraint name; an indeterminate
23505 falls back to that path, so no regression).
Item 2 (single source validator): strengthen parseSource into THE single form
validator for the source jsonb column -> returns a fully-valid RoleSource | null
(slug/language non-empty strings, version a number). The service's weaker
roleSource is removed and both layers share the RoleSource type (defined in the
db entity.types module both already import AiAgentRole from, so no import
cycle). normalizeRow / the read path now only ever yield a valid RoleSource or
null; a malformed stored source normalizes to null (tolerated by the service).
Tests: parseSource null for {} / {slug:123} / {slug:'a'} / empty-string keys /
string version, typed value for a full valid shape; service test that a
source-uniqueness 23505 is skipped (not errored) and the batch continues.
Verified the partial index rejects a duplicate source-not-null row but allows
two source-NULL rows, and the migration up/down run cleanly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- 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>
- Rename catalog-source migration 20260626T120000 -> T150000 so it sorts
after develop's latest migration (T140000-page-temporary-notes); the old
timestamp predated ai-chat-message-status/share-aliases and tripped
Kysely's #ensureMigrationsInOrder, aborting server boot.
- Provider: inject a Nest Logger and log the real cause (incl. response
status) in the parseJson / readLocal / fetchRemote catch blocks, and
propagate a useful cause into the BadGatewayException message; add a
shortError helper (robust to jest's realm-shifted Error-likes).
- Provider: replace the manual Uint8Array assembly with
Buffer.concat(chunks).toString('utf8'); keep the streaming size cap.
- Controller spec: add admin-gate coverage for the 4 catalog routes
(catalog/catalogBundle/import/updateFromCatalog) - non-admin Forbidden +
service untouched, admin delegates with the right args.
- Service spec: add getCatalog/getCatalogBundle tests covering the
localized() three-tier fallback, the sorted language union, the
missing-bundle BadGateway, and the role-version default.
- Provider spec: add remote fetch-rejects and non-ok (503) error branches.
- Service: drop the dead Date.now() tail in freeName (now an explicit
unreachable throw) and extract a shared isUniqueViolation() predicate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Admins can browse a curated catalog of agent roles, import roles/bundles
into a workspace, and update an imported role when the catalog ships a
newer version.
Catalog: a set of JSON files (index.json manifest + bundles/<id>/<lang>.json)
served from a local folder (dev) or a remote http(s) base URL via
AI_AGENT_ROLES_CATALOG_URL. Seeded with the existing 7 RU roles (editorial +
research bundles) plus EN translations.
Server:
- migration: nullable jsonb `source` column on ai_agent_roles
({ slug, language, version }; null => manually created)
- catalog provider: remote fetch with timeout + streaming size cap, or local
read; ^[a-z0-9-]+$ segment guard against path-traversal/SSRF
- admin endpoints: catalog, catalog/bundle, import, update-from-catalog
- import/update match by slug+language; update preserves `enabled`
Client:
- catalog modal with language selector and Import/Installed/Update states
- "Import from catalog" button + empty-state CTA in the roles settings panel
- en-US/ru-RU strings
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Remove the separate, un-toggleable `settings.ai.generative` workspace flag
(and its write-side alias `generativeAi`) along with the dead "Ask AI"
generative editor menu, and re-gate the AI page-title generation on the
general AI chat flag (`settings.ai.chat`) — the same toggle that enables
the chat agent and the chat stream endpoint.
Why: the `generative` flag had no UI toggle (its switch was already removed,
leaving orphaned i18n strings), so the title-generation button was
unreachable on self-hosted. The "Ask AI" menu was dead — its atom was never
rendered. Consolidating onto the AI chat flag makes the title button follow
the one AI switch users actually have.
Changes:
- server: title-gen endpoint gate generative -> chat (ai-chat.controller.ts);
remove generativeAi from update DTO and workspace service (update block,
delete line, cloud default now { ai: { chat: true } }); fix repo comment;
migrate generate-page-title spec assertions generative -> chat.
- client: title-gen gate -> settings.ai.chat (full-editor.tsx); remove the
dead Ask AI button + showAiMenu wiring from bubble-menu; remove AskAiGroup
usage/import and commented block from fixed-toolbar; delete ask-ai-group.tsx;
remove showAiMenuAtom; drop generative/generativeAi from workspace types.
- i18n: remove 3 orphaned generative-AI keys from all 12 locales.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On opening the floating AI-chat window from the header on a document page,
auto-open the LAST chat bound to that document. Binding reuses the existing
ai_chats.page_id (no migration): the bound chat is the requesting user's
most-recent non-deleted chat created on that page, so a new chat on the page
becomes the bound one for free. Resolution happens only on a genuine
closed -> open transition; the provenance badge deep-link is untouched.
Server: AiChatRepo.findLatestByPage + POST /ai-chat/bound-chat (BoundChatDto),
both read-only and owner/workspace-scoped.
Client: getBoundChat service + useOpenAiChatForCurrentPage hook wired into the
app-header entry point (fail-soft to a fresh chat; draft/role cleared only on a
real switch).
Tests: repo scoping/ordering, controller wiring, and hook behavior.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Register the new AI-chat keys "Send now" and "Interrupt and send now" in
both en-US and ru-RU catalogs so the UI never renders mixed-language
tooltip/aria-label (i18n policy).
- Make INTERRUPT_NOTE module-private (drop the unused re-export), matching the
module's private DEFAULT_PROMPT/SAFETY_FRAMEWORK siblings.
- Reset interruptNextSendRef in the flush-on-abort branch when nothing is
actually sent, so a stuck one-shot interrupt flag cannot tag the next
unrelated send; flushNext now reports whether it sent.
- Add a CHANGELOG [Unreleased]/Added entry for #198.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-turn model conversation was rebuilt via findRecent(chatId, ws, 50),
a sliding window that dropped the beginning of any chat longer than ~50 stored
rows. Switch streamChat to the existing findAllByChat, which loads the full
non-deleted transcript chronologically with a 5000-row memory-safety backstop
(keeps the newest rows + logs a warning on overflow) — a safety net, not a
conversational limit. Remove the now-unused findRecent method and update the
comments/log text that referenced it (findAllByChat now feeds both the Markdown
export and the model history).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an AI button in the page byline that generates a note's title from the
live editor content (including unsaved edits) and applies it immediately.
Server: one-shot, non-streaming POST /ai-chat/generate-page-title mirroring
the chat generateTitle path — gated by settings.ai.generative, throttled via
AI_CHAT_THROTTLER, resolves the workspace chat model and returns { title }.
The endpoint never touches the page; the client applies the title through the
existing /pages/update route (which enforces edit permission).
Client: ai-chat-service.generatePageTitle, a useGeneratePageTitle hook that
converts the editor HTML to markdown, calls the endpoint, applies the title
via updateTitle + updatePageData, reflects it in the unfocused title editor,
and broadcasts the UpdateEvent (mirroring TitleEditor.saveTitle). A sparkles
button (GenerateTitleGroup) renders next to dictation, edit-mode + flag gated.
Tests: pure cleanGeneratedTitle helper + controller gate/delegation/error-map.
i18n: en-US + ru-RU strings.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a "send now" button to queued AI-chat messages: it interrupts the
running agent and immediately sends that message, while the agent's
partial output at interruption is kept in history and the next turn is
marked as a user interrupt.
Client:
- queue-helpers: pure `promoteToHead` to move a queued message to the head.
- chat-thread: `sendNow` (promote head + abort + flush-on-abort), one-shot
`flushOnAbortRef`/`interruptNextSendRef`, `interrupted` flag in the
request body, and the "send now" ActionIcon in the queued list.
Server:
- `interrupted` on AiChatStreamBody; pure `isInterruptResume` confirms the
client hint against persisted history (prev assistant turn aborted/
streaming) before honouring it.
- prompt: INTERRUPT_NOTE injected in the context section only on a
confirmed interrupt-resume turn so the model treats the partial answer
above as incomplete.
Tests: promoteToHead, chat-thread send-now (abort + resend + one-shot
interrupt flag + non-streaming immediate send), isInterruptResume, and
the prompt interrupt-note injection.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
onFinish always receives a totalUsage object, so the `?? {}` guard and
optional chaining were dead. Extract the field-level extraction into a
recordTurnUsage method (totalTokens, else input+output) and unit-test that
recordShareTokens receives the summed value when totalTokens is absent and the
authoritative total when present.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The anonymous public-share assistant only capped the COUNT of requests
(100/hour/workspace), not their cost. One accepted turn runs the agent loop
up to stepCountIs(5), re-sending the whole client-held transcript as input on
every step, while maxOutputTokens caps only the output; the request window is
hourly with no daily ceiling, so a steady stream at the cap sustains ~24x its
count per day. Counting requests therefore does not bound the owner's LLM bill
(red-team finding #5).
Add a second cost contour: a cluster-wide, sliding-window per-workspace TOKEN
budget over a rolling day. It is checked read-only BEFORE a turn streams (429,
no request slot consumed, nothing spent) and the turn's real usage
(totalUsage: input re-sent per step + output, summed across all steps) is
recorded once it finishes via streamText onFinish. Fails closed on the check
(deny when Redis can't prove we're under budget); best-effort on the record.
Env-overridable via SHARE_AI_WORKSPACE_TOKEN_BUDGET_PER_DAY (default 1M/day).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In-app AI-chat tools used bare zod schemas, so when the model dropped a
required arg (typically pageId) in a parallel/batch tool call, the AI SDK
forwarded zod's raw "expected string, received undefined" text to the model
— not actionable. Add a centralized modelFriendlyInput(shape) wrapper that
keeps the exact JSON Schema contract (required/description/constraints via
z.toJSONSchema draft-7) but replaces the raw zod text with a message naming
each missing/invalid parameter and reminding the model not to drop ids like
pageId in parallel batches. No value is guessed/backfilled (cf. #159).
Applied to every in-app tool: the sharedTool() builder and all inline
inputSchema in ai-chat-tools.service.ts, plus public-share-chat-tools.service.ts.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The floating chat window's header badge flipped meaning — a live per-turn token
counter while streaming, the persisted context size at rest — so it "reset to 1"
on each prompt and conflated two different numbers. Replace it with a stable
"current / max" context badge (e.g. `572 / 200k`). The live "Thinking · N tokens"
inside the chat body stays; only the duplicate live counter is removed from the
header.
Max comes from a new admin setting "Context window (tokens)". The server resolves
it and attaches `maxContextTokens` to the completed assistant turn's metadata
(next to contextTokens), so the badge needs no client-side model resolution and
this survives public shares / per-role models.
Server:
- ai.types: chatContextWindow on AiProviderSettings + PROVIDER_SETTINGS_KEYS +
ResolvedAiConfig + MaskedAiSettings.
- workspace.repo: chatContextWindow in AI_PROVIDER_SETTINGS_ALLOWED (parity).
- update-ai-settings.dto: @IsInt @Min(0) chatContextWindow.
- ai-settings.service: coerce the ::text-stored value to a positive int in
resolve()/getMasked().
- ai-chat.service: flushAssistant writes metadata.maxContextTokens (>0); the
completed turn passes resolved.chatContextWindow.
Client:
- ai-chat.types: maxContextTokens on the message-row metadata.
- ai-chat-window: read maxContextTokens; render "current [/ max]"; drop the
liveTurnTokens state/branch and the onLiveTurnTokens prop; new tooltip.
- chat-thread: remove the live-turn-token throttle effect and plumbing.
- count-stream-tokens: drop the now-dead liveTurnTokens()/types; keep
estimateTokens.
- settings: chatContextWindow on IAiSettings(+Update) + a NumberInput in the AI
provider settings form.
i18n: add the badge/settings keys (en, ru); remove the two now-unused keys.
Tests: flushAssistant maxContextTokens, DTO validation, trim token tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Approve-with-comments re-review; no blockers. All 7 actionable points (8 is a
forward-looking architecture note — recommendation A, keep as-is):
1. chat-markdown.util spec: restore parity coverage of the removed client spec —
tool error state (+ errorText), unknown-tool fallback (`Ran tool <name>` en /
`Выполнил инструмент <name>` ru), and the circular-output stringify catch.
2. findAllByChat row cap is now testable (injectable limit) + an int-spec proves
truncation on a modest volume.
3. Stability: the per-step durability updates are SERIALIZED via a promise chain
(stepUpdateChain) so they commit in step order — onlyIfStreaming already
closed the finalize race, this closes inter-step ordering.
4. findAllByChat keeps the NEWEST messages on truncation (order DESC + reverse,
like findRecent) and logs a warning with chatId, instead of silently dropping
the newest tail.
5. The LABELS parity comment already references the real path (tool-parts.tsx /
toolLabelKey) — confirmed accurate.
6. Removed the redundant 'off-by-one boundary' test (strict subset of the two
adjacent prepareAgentStep cases).
7. Extracted the terminal-finalize dispatch into a shared `applyFinalize`, used
by BOTH the service's finalizeAssistant and its test — the test now exercises
the real path, not a copy, so a production drift fails it.
Verified: server build + 325 ai-chat unit + 6 integration; prettier clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
15-point review of the persistent-history PR. Architecture decisions: crash
recovery = recency threshold; tool-label duplication = leave as-is.
Must-fix:
1. Boot-sweep bounded by recency. sweepStreaming now also requires
`updatedAt < now() - SWEEP_STREAMING_STALE_MS` (10 min), so a fresh replica's
startup sweep can't abort a turn another replica is actively streaming
(multi-instance deploy). Int-spec: a FRESH 'streaming' row is NOT swept, a
STALE one IS.
2. Restore export during the FIRST streaming turn of a new chat (#174). The
server chatId is now adopted EARLY (in-place, on the start-chunk metadata) via
a new `onServerChatId` callback wired through use-chat-session → chat-thread,
so `activeChatId` is set at turn start and the Copy button is live mid-first-
turn (canExport = !!activeChatId). Hook tests for early/in-place/no-op adopt.
3. Cover finalizeAssistant's fallback-insert branch: extracted pure
`planFinalizeAssistant(assistantId)` (update when id present, insert when the
upfront insert failed) + a dispatch harness test for both arms.
Tests: onModuleInit lifecycle spec (sweep called; throw → resolves + warns);
int-spec updatedAt assertion → toBeGreaterThan.
Cleanups: cap findAllByChat at 5000 rows; upfront-insert-failure log carries
chatId+workspaceId; removed the now-dead buildPartialAssistantRecord (only the
spec consumed it; shapes still pinned by the flushAssistant suite); controller
passes `lang: dto.lang` (normalizeLang handles undefined); dropped a no-op
`?? undefined` in errorOf; documented the content-column semantics change
(concatenated step text, UI renders from metadata.parts); CHANGELOG [Unreleased]
entry (#183, #174); reworded the stale LABELS parity comment.
Verified: server build + 323 ai-chat unit + 5 integration; client tsc + 160
ai-chat unit; prettier clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8-point multi-aspect review of the batch PR; security/regressions were clean.
1. Lease leak: the #180 reorder moved `toolsFor` (which leases external MCP
clients, refCount+1) ahead of buildSystemPrompt + forUser, but the only
release (closeExternalClients) was bound to the streamText callbacks. A throw
in between leaked the lease (refCount stuck, undici sockets held until
restart). Define closeExternalClients right after the lease and wrap
buildSystemPrompt+forUser in try/catch that closes-then-rethrows.
2. Cover the patch_node/delete_node dup-id refusal (#159#6): extract the guard
into a pure `assertUnambiguousMatch` (node-ops) and unit-test 0/1/>1.
3. Regress the body-before-title order (#159#10): mock-HTTP test (collab fails
fast against a server with no WS upgrade) asserts /pages/update (title) is
NEVER posted when the body write fails — for updatePage AND updatePageJson.
4. CHANGELOG [Unreleased]: #180, #168 (Added); #163 (Fixed).
5. Add the missing en-US i18n keys (Back to references / {{label}}).
6. Drop the duplicate content/empty/blank cases in ai-chat.prompt.spec.ts (they
repeat the buildMcpToolingBlock unit tests); keep only sandwich placement +
both-safety-copies.
7. CI Postgres pg16 -> pg18 (match docker-compose).
8. jsonb decode seam: shared `parseJsonbValue(value, guard)` in database/utils.ts
holds the legacy double-encoding self-heal in one place; parseToolAllowlist /
parseModelConfig keep only a type-guard.
Verified: server build + 124 unit + 15 integration; mcp 311; prettier clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The client sends the "current page" as { id, title } in the request body and the
server echoed BOTH verbatim into the system prompt context and the
getCurrentPage tool. id and title are independently attacker/desync-controllable
(two tabs, stale navigation), so openPage.id could point at page B while
openPage.title said "Page A" — the model then reported "updated Page A" while it
actually edited page B (CASL still allowed it; the user has access). Red-team
finding #4.
Resolve the open page ONCE against the DB via a new `resolveOpenPageContext`:
workspace-scoped lookup + access check, returning the AUTHORITATIVE { id, title }
(title from the DB row, never the client) or null (fail-closed) for a missing /
foreign / inaccessible page. That validated value now feeds the system prompt,
the getCurrentPage tool, AND the new-chat history origin (which previously did
this validation inline, for the id only — now shared, and the title is fixed
too).
Tests: resolveOpenPageContext covers no-id, not-found, foreign-workspace,
Forbidden, non-Forbidden-fault (fail-closed), the DB-title-wins-over-client case,
and null-title coercion.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_coder
claude_code
vvzvlad
a