- ai-chat: drop the unused pagePermissionRepo injection from
PublicShareChatToolsService (its only use moved into
ShareService.resolveReadableSharePage); update all 5 positional
test construction sites to match the 3-arg constructor.
- env: correct the anonymous share-AI per-workspace cap comment —
the limiter FAILS CLOSED on Redis failure (#62), not open.
- docs: sync README.ru.md with README.md — move "Page templates"
from Planned to Done and drop the dead plan-doc link.
Remaining test-coverage gaps tracked as #102, #103, #104, #105, #106.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The three collect*FromPmJson collectors shared the same recursion (and the #55
depth cap) but were copy-pasted, so a future edit could diverge them. Extract a
generic collectNodes(doc, {type, map, key, lastWins, skipChildrenOfType}) and
reimplement all three on it, byte-output-identical (transclusions last-wins;
references/embeds first-wins + transclusionSource skip). Documents (not removes)
the write-only page_template_references graph and the near-duplicate client
lookup-context as tracked follow-ups, per the issue's guidance.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The '(shareId,pageId) -> usable non-restricted page in THIS share' boundary was
written as 3 must-be-identical async sequences. They weren't: the chat funnel
omitted an explicit page.deletedAt check (latently safe via getShareForPage's
CTE) and layered isSharingAllowed separately. Add ShareService.resolveReadable-
SharePage(shareId,pageId,workspaceId) running the single canonical sequence
(getShareForPage -> id match (skipped when null) -> findById -> !deletedAt ->
!hasRestrictedAncestor) returning {share,page}|null; getSharedPage, the funnel,
and the getSharePage tool all use it. hasRestrictedAncestor now lives in the one
method no caller can skip; the funnel still returns uniform 404s and keeps
isSharingAllowed. Adds a direct security-invariant test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
resolveRoleForRequest and resolveShareRole duplicated the security invariant
'role exists, not soft-deleted, enabled, workspace-scoped, else null'. Move it to
AiAgentRoleRepo.findLiveEnabled(id, workspaceId) (deletedAt IS NULL + enabled +
workspace scope) and have both services call it, preserving each one's roleId
derivation + null handling. (describeProviderError half of #95 was done earlier.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 4-step html-embed gate (feature-enabled AND role-allowed -> stripHtmlEmbedNodes)
was replicated across call-sites, pinned only by brittle source-regex tests. Add
stripHtmlEmbedIfNotAllowed(json, {featureEnabled, role, onStrip}) and migrate the
5 plain strip-all sites (collab handler, page create+duplicate, both import paths,
transclusion) to it, each keeping its own feature/role resolve + log via onStrip.
Left the 2 sites with different semantics: persistence.extension (#29 preserve-
admin) and share.service (feature-only kill-switch, no role gate). Real unit tests
replace the regex pins; behavior identical.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
16 suites were disabled via testPathIgnorePatterns due to two root causes: lib0
ESM not transformed (the @hocuspocus/server -> lib0/decoding.js chain) and stock
'should be defined' specs built via Test.createTestingModule without providers.
Add lib0 to transformIgnorePatterns; convert the 14 DI placeholders to direct
new X(...) instantiation with stub deps (keeping a real construct smoke test);
re-enable the suites. Also updates the public-share limiter test to assert the
fail-closed behavior from #62 (surfaced now that the suite runs). Full server
suite: 67 passed, 689 tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Assessment of the page-embed depth/cycle cap: the server /pages/template/lookup
returns FLAT single-level content and does NOT recurse into embedded pages — the
recursive expansion + the PAGE_EMBED_MAX_DEPTH cap are entirely client render
concerns, and a scripted client is already bounded by the per-user throttle
(30/min) + the ArrayMaxSize(50) per-call cap. So no server-side depth guard is
needed; documented at lookupTemplate so future readers don't add a redundant one
or assume server recursion exists.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
handleMessage became a no-op and PageWsListener intentionally ignored
PAGE_UPDATED, so a rename/icon change (client operation:updateOne) was no longer
rebroadcast -> other clients saw stale title/icon in the sidebar+breadcrumbs
until a reload (create/duplicate/restore were covered; updateOne regressed).
Add a server-authoritative onPageUpdated handler: PageService.update detects a
real title/icon change (DTO carries the field AND value differs; no-op/content-
only saves excluded) and attaches a treeUpdate snapshot to PAGE_UPDATED; the
listener broadcasts a tree updateOne via the restriction-aware emitTreeEvent
(so a restricted page's title never leaks). Content-only saves attach nothing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sidebar-pages-tree.spec tested a LOCAL COPY of the tree-shaping (so a regression
in the real getSidebarPagesTree was invisible) and justified it with a false
jest-config claim (the ^src mapping exists). Extract the pure shaping into
shapeSidebarPagesTree(); the service now calls it and the spec imports the REAL
helper. Behavior unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The concurrent-soft-delete guard was already covered; add the missing assertion
that update() returns toView(updated) from the post-update re-fetch (full
AgentRoleView shape, distinct second findById row), so a regression returning the
stale pre-update view or leaking columns is caught.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The anonymous share page-fetch tool's positive branch (sanitize via
updatePublicAttachments then jsonToMarkdown before returning to the model) was
untested, so a dropped/reordered sanitizer would ship a comment-mark/raw-
attachment leak with green tests. Add a positive-branch test pinning the
sanitizer call + that markdown derives from sanitized content, and a soft-deleted
test asserting a generic error with no content fetch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
chatModel was a free string accepted with empty/garbage values, failing only at
runtime as a provider 503; tighten it (trim + non-empty + max 200). Driver was
already @IsIn(AI_DRIVERS). Collapse the client driver list to one AI_DRIVER_VALUES
source and add a contract test that reads the server AI_DRIVERS and fails on
client/server drift.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
collectPageEmbedsFromPmJson (and the sibling collectors/remap) recursed with no
guard, so a pathological/cyclic non-JSON input could stack-overflow (RangeError).
Add a depth cap (1000, far above any real doc nesting) so such input is handled
gracefully. Normal documents are unaffected. Updates a stale test that asserted
the old throwing behavior.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A custom AI-role's text preceded the only SAFETY_FRAMEWORK block and replaced
the persona, so a jailbreak in the role text sat before the safety rules.
buildSystemPrompt now emits SAFETY both before AND after the persona, with the
role/persona delimited as lower-trust (<role_persona note=...>); the default
persona is sandwiched too. Context (currently-viewing-page) preserved.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#67: movePage didn't check the destination wasn't the page itself or inside its
own subtree, so MCP/REST/agent/fast-drag could persist+broadcast a cycle. Reject
before the update (self-parent, or moved page among the destination parent's
ancestors via getPageBreadCrumbs).
#64: movePage emitted PAGE_MOVED from a stale pre-read even when the row didn't
change / was concurrently deleted (phantom move). Gate the emit on
updateResult.numUpdatedRows !== 0n. Both are movePage hardening in one method.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#60: streamText had no maxOutputTokens, so one anonymous request could run up
the provider bill. Add maxOutputTokens (env SHARE_AI_MAX_OUTPUT_TOKENS, default
512) via resolveShareAiMaxOutputTokens().
#95: the anonymous path hand-built error strings, diverging from the unified
describeProviderError format used on the authenticated path; both onError blocks
now call describeProviderError so a share reader sees 402/429/503 causes in the
same form (and the stack is still logged). Both changes are in this one file and
share hunks, hence one commit.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A user with password=NULL passed the missing/disabled guard and reached
comparePasswordHash(pw, null), which native bcrypt rejects -> 500 on
/api/auth/login and, on /mcp, a leaky 401 that the brute-force limiter ignored
(enumeration oracle + limiter evasion). Treat a null/empty password like a
missing user in verifyUserCredentials (dummy compare for timing parity + unified
CREDENTIALS_MISMATCH_MESSAGE) and reject early in changePassword before bcrypt.
Contract spec asserts the null-password guard.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MAX_SHARE_MESSAGE_CHARS only counted text parts, so a forged non-text part
(tool-result/file/data) bypassed the cap and bloated the model input
(token-DoS); convertToModelMessages would also expand a forged tool-result. The
anonymous path runs no tools, so a client non-text part is never legitimate —
reject any message with a non-text part (isTextUIPart) before the size check.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The per-workspace anonymous share-AI cost cap failed OPEN on a Redis error
(return true => admit), so a Redis outage removed the cap entirely (unmetered
billable anonymous calls). The feature is optional, so unavailability is
harmless: fail CLOSED (return false => controller 429s) instead.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve the html-embed.spec.ts conflict as a union: both #46 and #49 (already in
develop) added different test cases to the same file. Keep all of them —
stripHtmlEmbedNodes gets #46's root-node case plus develop's deeply-nested,
non-object and empty-content cases; #46's collectHtmlEmbedSources and
stripDisallowedHtmlEmbedNodes suites and develop's hasHtmlEmbedNode suite all
kept; imports unioned. No production code conflicted.
Full suite green: server 651, client (16 files), editor-ext 56, mcp 247.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve conflicts from the parallel page-embed refactor that landed in develop
via #49:
- page-embed-view.tsx: keep develop's canonical decideEmbedState for the
cycle/depth/availability guard; keep #45's #39 chrome cleanup (single source
link, IconFileText fallback) and #40 refresh remount key. Drop #45's now-unused
isPageEmbedCycle/isPageEmbedTooDeep wiring.
- page-embed-picker.tsx: use develop's excludeHost util; drop #45's duplicate
filterPageEmbedOptions and its test.
- page-embed-ancestry-context.test.tsx: keep #45's superset suite.
- page-template-access.spec.ts: keep develop's constructor args; update the two
deleteByReferenceAndSources assertions to the new 4-arg workspace-scoped
signature introduced by #45 (#36 defense-in-depth).
Full suite green: server 624, client 219, editor-ext 56, mcp 247.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add ~330 tests across server (Jest), client (Vitest), editor-ext (Vitest)
and packages/mcp (node:test) for the gitmost features added since
053a9c0d: AI chat, AI agent roles, public-share assistant, MCP per-user
auth, HTML embed, page templates/embed, realtime tree, tree
expand/collapse, and the AI-settings UI.
Test-tooling fixes (prerequisite, were silently hiding coverage):
- Repair 3 page-template specs broken by the 11-arg TransclusionService
constructor; they never compiled, so template access-control / content
-leak / unsync-strip coverage was fictitious.
- Build @docmost/editor-ext before server tests via a `pretest` hook;
the stale dist omitted the new HtmlEmbed/PageEmbed exports (TS2305).
- Let jest resolve the .tsx email templates: add `tsx` to
moduleFileExtensions and widen the ts-jest transform to (t|j)sx?.
Behaviour-preserving "extract pure core" refactors that the tests drive:
- server: resolveShareAssistantRequest + uiMessageTextLength
(public-share controller), decideBasicGate + mapAuthResultToResponse
(mcp), buildErrorAssistantRecord (ai-chat), jsonbObject export (roles).
- client: render-raw-html + shouldExecute/canEdit, decide-embed-state,
page-embed picker utils, tree-socket reducers, open/close branch maps,
isEndpointConfigured/resolveKeyField; buildTreeWithChildren now treats
a permission-trimmed orphan as a root instead of crashing.
Deferred (need a test DB or HTTP harness, documented in the specs):
repo-level Postgres integration tests and the public-share XFF E2E.
Pre-existing DI/lib0-ESM suite failures are untouched and out of scope.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Post-merge hardening from the #13 security review:
- isInitializeRequestBody now delegates to the SDK isInitializeRequest (same
predicate as packages/mcp/http.ts), so a bare {method:'initialize'} with no
id/params no longer triggers the side-effecting login() (audit-spam /
user_sessions growth) before http.ts 400s it.
- Bind the Bearer path to the instance workspace: verifyBearerAccess rejects a
token whose payload.workspaceId != the instance workspace (resolved via
workspaceRepo.findFirst, consistent with the Basic path); optional param so
it's a no-op when unset.
- Close the user-enumeration timing oracle in verifyUserCredentials: the
missing/disabled branch now runs a bcrypt compare against a module-level dummy
hash whose cost (12) matches production saltRounds, so both paths take one
equal-cost bcrypt compare; the exact CREDENTIALS_MISMATCH_MESSAGE is preserved.
- Document the trusted-proxy requirement for the spoofable per-IP brute-force
limiter in .env.example (trustProxy is on; deploy behind a trusted proxy).
- Add real-execution coverage for enforceBasicLoginGate (SSO enforced / EE-MFA
bundled vs not / user-MFA / workspace-enforced-MFA) instead of stubbing the gate.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The create/duplicate/import gate tests asserted gate presence via brittle
expect(SRC).toMatch(/regex/) over the source text plus a reimplemented
applyGate() stand-in, so a refactor could break the real gate while they still
passed. Rewrite both specs to execute the REAL methods (PageService.create /
duplicatePage; ImportService.importPage; FileImportTaskService.processGenericImport)
with each caller role and assert on the PERSISTED content via hasHtmlEmbedNode:
member -> stripped, admin/owner+toggle ON -> preserved, toggle OFF -> stripped
for everyone, unknown/missing role -> fail-closed. No source-regex assertions
remain.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- The security-relevant catch->not_found branch in lookupTemplate (returns
not_found instead of raw content when comment-mark stripping throws) is now
tested by forcing the strip to throw with a malformed text node, asserting no
content/marks leak.
- not_found for a soft-deleted source resolved through the REAL
filterViewerAccessiblePageIds (deletedAt-excluded), not the stubbed filter.
- Rename the misleading 'honours <=50 cap' test to reflect it only exercises
dedup (the cap lives in the DTO, never engaged in the service unit).
- Cover the onlyTemplates search filter (restricts to is_template=true).
Also fix two pre-existing failing 'should be defined' specs (search service +
controller) that couldn't resolve the @InjectKysely token via createTestingModule.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the per-node pageEmbed remap decision into a shared pure helper
(remapPageEmbedSourceId) and use it BOTH in PageService.duplicatePage and the
JSON walker, so the test guards the real production path (not a mirror that
could drift). Behavior is identical: source in the copied set -> new copy id;
otherwise keep the original. Add jest coverage (16 tests): the remap helper
(in-set/out-of-set/null/nested), syncPageTemplateReferences toDelete (stale refs
removed with the right workspaceId), and insertTemplateReferencesForPages
multi-workspace grouping/filtering.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The transclusion specs predated two added constructor params, so they failed to
compile (TS2554: expected 11 args, got 10) and the suites couldn't run. Add the
missing mock args: workspaceRepo (param 11) in the lookup/access specs, and
pageTemplateReferencesRepo (param 4, which had shifted pageRepo into the wrong
slot) in the unsync-html-embed spec. All three suites now compile and pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The current page id was only injected as text in the system prompt, which a
proxy (CLIProxyAPI) can rewrite/truncate, so the agent could lose track of 'this
page'. Add a getCurrentPage tool the model can call to read the open page (id +
title) from the server-side request context (forUser now takes openedPage,
threaded from body.openPage — the same value used for the system prompt). The
inline system-prompt line is kept as belt-and-suspenders. Reads/writes still go
through the CASL-enforced page tools by id, so this is strictly not worse than
the existing prompt hint — just delivered over a channel the proxy can't mangle.
User-approved on the issue. Completes #43 together with the hardness-1 fix.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Completes the workspace htmlEmbed kill-switch. The public-share path already
strips at serve time when the toggle is OFF, but the authenticated read paths
(/info and /history/info) returned page/history content with embeds intact, so
a disabled feature kept executing for in-workspace view-only viewers until the
page was next saved. Now both paths resolve the workspace toggle and run
stripHtmlEmbedNodes when it's OFF (fail-closed on a missing workspace), before
any markdown/html format conversion. Admin-authored content only — completeness,
not privilege escalation. Injects WorkspaceRepo into PageController.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Consistency hardening from #17 review (not currently exploitable):
- toggleTemplate now explicitly rejects a page outside the caller's workspace
(page.workspaceId !== user.workspaceId -> NotFound, avoiding existence leak)
instead of relying solely on the space-membership model.
- PageTemplateReferencesRepo.deleteByReferenceAndSources is now workspace-scoped
(adds a workspaceId filter + param), matching the 'scope by workspaceId
everywhere' invariant; the sole caller threads its workspaceId.
The PAGE_TEMPLATE_THROTTLER limit is intentionally left as-is (the issue's
throttle item was 'consider only'; no change without usage data).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace bare //@ts-ignore (no space, no reason) with // @ts-expect-error plus a
reason on the pageEmbed sourcePageId reassignment, matching the codebase style.
ProseMirror Attrs is read-only typed, so the reassignment genuinely errors —
@ts-expect-error is valid here.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Anonymous public-share AI assistant:
- Add a workspace setting `publicShareAssistantRoleId` so an admin can pick which
agent role (identity/persona) the anonymous assistant adopts. The role's
instructions REPLACE the built-in persona while the immutable safety framework
is still always appended; the role's optional model override takes precedence
over the cheap publicShareChatModel. Resolved server-authoritatively
(workspace-scoped, soft-delete aware; disabled/missing roles fall back to the
built-in persona, so the tool scope remains the real security boundary).
- Plumb the field through the update DTO, ai-settings service, the workspace.repo
ALLOWED whitelist, resolve()/getMasked(), stream-time role resolution and the
prompt/model, plus the settings UI: a new "Assistant identity" Select listing
enabled roles (and surfacing a saved-but-disabled role explicitly).
Public-share branding / floating icon:
- Fix the AI assistant FAB overlapping the "Powered by ..." button (both were
Affixed bottom-right): stack the FAB above the bottom-right branding.
- Rename "Powered by Docmost" -> "Powered by Gitmost" and point the link at the
gitmost repo.
Tests: extend public-share-chat.spec (role persona replacement still appends the
safety framework, resolveShareRole edge cases, model-override precedence).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The html-embed feature toggle was enforced CLIENT-side in the NodeView (reads
settings.htmlEmbed from the logged-in workspace), so an anonymous public-share
viewer — who has no workspace context — always saw it as OFF and got a
placeholder instead of the executing embed. That broke the whole point (a
tracker must run for anonymous visitors).
Make it server-authoritative:
- share.service prepareContentForShare (the single path both share-content
flows use) strips htmlEmbed from served content when the workspace toggle is
OFF; both callers (updatePublicAttachments host page + lookupTransclusionForShare)
resolve the toggle once and pass it. Fail-closed: missing workspace -> OFF ->
stripped.
- NodeView executes whatever it was served in read-only/share mode
(shouldExecute = !editor.isEditable || htmlEmbedEnabled); the disabled
placeholder now only shows in the editable editor when OFF.
Net: anonymous share + toggle ON -> server serves the (admin-authored) embed ->
it executes for everyone; toggle OFF -> stripped server-side from every
share-content path (true kill switch); a non-admin embed can never be served
(save-path strip). No XSS regression in the editable editor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- make addTreeNode receivers idempotent (invalidateOnCreatePage guard +
buildTree dedup) so the author's self-echo no longer duplicates the node
- broadcast realtime tree updates for bulk copy/duplicate and import via a
root refetch: PAGE_CREATED now carries spaceId and the WS listener falls
back to refetchRootTreeNodeEvent when no per-node snapshot is present
- remove the now-dead client-relay inbound path (isTreeEvent/handleTreeEvent)
that remained a stale-restriction-cache attack surface
- honest string|null cast for a root move's parent id
- add tests: buildTree dedup; onPageCreated per-node vs refetch branching
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The admin-only raw HTML/JS embed is a deliberate stored-XSS surface, so gate the
whole feature behind a workspace toggle that is OFF by default; it only works
when a workspace admin explicitly enables it.
- settings.htmlEmbed (boolean, default false) + workspace-update field htmlEmbed,
persisted via WorkspaceRepo.updateSetting with an audit diff. Flipping it is
admin-only (same Manage Settings CASL as other workspace toggles).
- New gate htmlEmbedAllowed(featureEnabled, role) = featureEnabled && admin/owner.
All 7 server write paths (create, duplicate, collab onStoreDocument, REST/MCP/AI
updatePageContent, single + zip import, transclusion unsync) now read the
workspace's settings.htmlEmbed and strip unless (toggle ON AND admin). OFF
(default, or a failed/empty workspace lookup) strips htmlEmbed for EVERYONE
including admins -> existing embeds are cleaned up on next save, none persist.
- Client (defense-in-depth): the /html slash item is hidden unless toggle ON +
admin; the NodeView executes nothing and shows a 'disabled in this workspace'
placeholder when OFF; an admin Switch in Workspace Settings -> General with a
description of the behavior.
- docs/html-embed-admin.md documents the toggle + admin-only + fail-closed
coedit (a non-admin save strips an admin's embed) + execution semantics.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve conflicts with the independently-merged ai-agent-roles feature:
- ai-chat.module.ts: keep BOTH AiAgentRolesModule and the public-share
wiring (Share/Search modules, PublicShareChatController, services).
- ai.service.ts: take develop's getChatModel ChatModelOverride superset,
which already covers the public-share model-id-only override.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up fixes on the agent-roles feature:
- ai.service: a cross-driver override to the ollama driver (when the
workspace driver is not ollama) now fails with an explicit 503 instead
of silently reusing the workspace base URL, which belongs to a different
provider. Same-driver ollama and openai/gemini overrides are unchanged.
- migration: add a partial unique index on (workspace_id, name) WHERE
deleted_at IS NULL so role names are unique per workspace without
soft-deleted rows blocking re-creation; map Postgres 23505 to a 409
ConflictException on create/update.
- dto: validate the role id as @IsUUID instead of @IsString.
- roles list: do not expose instructions/modelConfig to non-admin members.
The list endpoint now returns a picker view (id/name/emoji/description/
enabled) to members and the full view only to admins (same gate as the
CRUD endpoints). Client IAiRole fields made optional accordingly.
Adds tests for the cross-driver-ollama throw, the 23505->409 mapping, and
the non-admin picker-view security invariant.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Integrate the already-merged step-limit work from develop. Only conflict was
ai-chat.service.spec.ts: both sides appended a describe block and edited the
import line. Resolved as a union — keep compactToolOutput + the assistantParts/
serializeSteps/rowToUiMessage suites (this branch) AND the prepareAgentStep
suite (develop), importing all symbols from ai-chat.service.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds explicit isIpAllowed cases for the CGNAT, ULA (fd00::/8) and IPv4-mapped
IPv6 loopback (::ffff:127.0.0.1) sample addresses from the parallel
safety-coverage branch. The mapped-loopback case is genuinely new (the existing
table only covered the mapped *private* variant); CGNAT and ULA ranges were
already covered with other samples and are kept here as explicit regression
guards for these specific addresses.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Behaviour change (split out of the test commit per review, and now covered).
Both the stream onError log line and the error text streamed to the client were
formatted by separate inline blocks that only emitted "<status>: <message>".
Route both through the shared describeProviderError() so formatting stays in one
place.
BEHAVIOUR CHANGE: describeProviderError additionally appends a single-line,
300-char-truncated snippet of the provider responseBody/text. So the log line
AND the user-facing stream error now include that snippet (e.g. the HTML error
page from a misconfigured endpoint), which previously neither did. This is
intentional — it makes a misconfigured external endpoint diagnosable — and is
safe: the API key travels in the Authorization header and is never echoed in
the response body (see the util's docstring). A `fallback` param is added so
each call site keeps its own default ('AI stream error' for the stream).
Adds ai-error.util.spec.ts covering the formatter, including the appended /
truncated body snippet, so this behaviour is no longer untested.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Unit tests for the safety-critical paths: crypto secret-box (round-trip,
tamper detection, wrong key), the SSRF guard (blocked ranges + DNS-rebinding),
the ai-chat tools service, the page-embedding repo, and the
assistant-parts/serialization helpers. Those server helpers (assistantParts,
rowToUiMessage, serializeSteps) are exported ONLY for the tests — no runtime
change.
Also: keyboard a11y on the chat history header and conversation rows
(role/tabIndex/Enter+Space), and DRY refactors that move shared logic into one
place (isToolPart -> tool-parts util; buildInitialValues in the MCP form).
The behaviour-changing edits that previously rode along in this commit are
split out into the following two commits, per review.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
