Reconcile the diverged develop (13 ahead / 20 behind) with gitea/develop.
Conflict resolution — html-embed: keep the local sandboxed-iframe model
(opaque-origin srcdoc, no role-gating) and supersede gitea's same-origin
strip/kill-switch hardening (#26/#28/#29/#30). The 4 conflicted html-embed
source files resolve to the local version; the 3 strip-era spec files stay
deleted. The strip apparatus (stripDisallowedHtmlEmbedNodes,
collectHtmlEmbedSources, canAuthorHtmlEmbed, htmlEmbedAllowed) is fully gone.
Integrate gitea's page-templates / page-embed work (#31-#40) cleanly.
Fix an auto-merge arity mismatch: two new gitea page-template specs
constructed TransclusionService with the pre-sandbox 11-arg signature; drop
the trailing workspaceRepo argument to match the reduced 10-arg constructor.
Verified: server + client tsc --noEmit clean; jest (html-embed + transclusion)
14 suites / 119 tests passing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Deduplicate the "save a workspace setting" plumbing shared by HtmlEmbedSettings
and TrackerSettings (workspace atom read, isLoading state, updateWorkspace + atom
merge forcing settings[key], success/error notifications) into a new
feature-scoped hook useWorkspaceSetting(key).
- Each component keeps its own interaction model: html-embed is an optimistic
toggle with revert-on-failure; tracker is edit-then-save on an explicit button.
- Unify error handling on the better pattern: surface err.response?.data?.message
and use console.error (html-embed previously used console.log + a generic message).
No user-facing behavior change; client typecheck clean.
Test-coverage follow-ups (untested trackerHead injection in ShareSeoController and
the no-op audit branch) tracked in #100.
Follow-up fixes to the htmlEmbed-sandbox / trackerHead change:
- share-seo: inject trackerHead via a function replacer so `$`-sequences
($&, $', $`, $$) in the admin snippet are inserted literally instead of
being treated as String.replace substitution patterns; warn when the
</head> marker is absent instead of silently skipping injection.
- mcp: register a passthrough `htmlEmbed` node in the schema mirror so an
AI/MCP edit of a page containing an embed no longer throws
"Unknown node type: htmlEmbed" in TiptapTransformer.toYdoc.
- editor-ext + client: treat a non-finite `data-height` as auto (null) so a
crafted/corrupted height cannot disable auto-resize or yield a NaN iframe
height; extract a shared clampHeight helper.
- client: rename render-raw-html.{ts,test.ts} -> html-embed-sandbox.{...} and
shouldExecute -> shouldRender so the seam name matches the sandbox model.
- client: i18n the iframe title; surface the real error reason in
tracker-settings (console.error + err.response.data.message).
- docs: note hasHtmlEmbedNode is now a test-only helper; add an Unreleased
CHANGELOG entry; drop the dangling "arbitrary HTML embed" planning-doc ref.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the htmlEmbed node from same-origin raw-HTML execution to a sandboxed
iframe (sandbox="allow-scripts allow-popups allow-forms", no allow-same-origin,
srcdoc) with postMessage auto-resize (validated by event.source) and an optional
manual height attr. The block now runs in an opaque origin and cannot reach the
viewer's cookies/session/API, so it is safe for any member.
Because the block is now harmless, remove the entire admin/role gating apparatus:
drop htmlEmbedAllowed/canAuthorHtmlEmbed/stripDisallowedHtmlEmbedNodes/
collectHtmlEmbedSources and every role-based strip on the write paths (collab
REST/MCP + socket, page create/duplicate, import x2, transclusion unsync), along
with the now-unused WorkspaceRepo/UserRepo injections and the PageService.create
callerRole param. Keep one strip: prepareContentForShare still removes htmlEmbed
on the anonymous public-share read path when the workspace master toggle is OFF.
The workspace settings.htmlEmbed toggle is now a plain feature switch (gates the
slash-menu and share rendering); when ON the block is available to all members.
Add settings.trackerHead: an admin-only raw HTML/JS analytics snippet injected
verbatim into the <head> of public share pages only (ShareSeoController), for
trackers that genuinely need same-origin. Admin-gated via the existing CASL
Manage/Settings ability; never injected into the authenticated app shell.
Closes security-review findings #1, #2, #4, #5, #10 (and #3 as a security issue).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Harden the anonymous public-share AI assistant against token-cost abuse
before exposing it to the internet:
- Add an env-tunable per-request output ceiling (maxOutputTokens) to the
public-share streamText call so one anonymous request cannot run up the
provider bill even if the per-IP throttle is evaded. New
resolveShareAiMaxOutputTokens() / SHARE_AI_MAX_OUTPUT_TOKENS_DEFAULT
(env SHARE_AI_MAX_OUTPUT_TOKENS, default 512), mirroring
resolveShareAiWorkspaceMax().
- Flip the per-workspace cost limiter to FAIL CLOSED on Redis failure
(was fail-open): if Redis is unavailable we cannot prove the workspace is
under its cap, so deny rather than admit an unmetered, billable call.
- Update the limiter spec (fail-open -> fail-closed) and add resolver tests;
document both knobs in .env.example.
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>
After develop merged, mcp.service.ts calls decideBasicGate from mcp-auth.helpers.
The gate spec mocked the whole module returning only FailedLoginLimiter, so the
merged code crashed with 'decideBasicGate is not a function' (7/7 failing).
Spread jest.requireActual('./mcp-auth.helpers') so the real helpers are kept and
the gate exercises real logic; keep only FailedLoginLimiter stubbed so its
constructor runs without a real sweep timer.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add .github/workflows/test.yml (pnpm + Node 22): on pull_request and push
to develop it installs, builds @docmost/editor-ext and runs `pnpm -r test`
across all packages (server Jest, client Vitest, editor-ext Vitest,
packages/mcp node:test). So tests now run automatically in CI, not just
on demand.
To make the run green, quarantine the 16 pre-existing stock NestJS
`should be defined` scaffold specs via jest `testPathIgnorePatterns` —
they never compiled (missing DI providers / lib0 ESM) and assert nothing
useful. Tracked for a proper fix/removal in issue #56. Verified each
pattern drops only its scaffold (46 of 62 suites still collected) and the
full `pnpm -r test` is green: server 587, client 185, 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>
A non-admin's transient htmlEmbed could execute in other open editors until the
debounced (10s) onStoreDocument strip. Add a ~300ms onChange-debounced early
strip (guardHtmlEmbed) that converges the shared ydoc for everyone far sooner.
Safety-critical details:
- Scheduled from onChange ONLY for non-admins AND only when the workspace toggle
is ON (cached per-document in onLoadDocument), so the common toggle-OFF case
does zero extra work.
- guardHtmlEmbed does ALL async work (toggle + persisted allow-list read) FIRST,
then performs fromYdoc -> strip -> fragment.delete -> applyUpdate in a single
SYNCHRONOUS, await-free block, so no inbound Yjs update can interleave and a
concurrent edit can never be clobbered. Bails if document.isDestroyed.
- Reuses the #29 preserve logic (admin-vetted embeds survive; only the non-admin's
new ones are stripped). Loop-safe (corrective update has null origin -> no
reschedule; post-strip no embed -> cheap no-op). Per-document timer cleared on
unload. onStoreDocument stays the authoritative backstop.
The irreducible residual is only the very first inbound broadcast before the
debounce fires — Hocuspocus exposes no synchronous beforeBroadcast filter.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The collab persist strip keyed to the storing connection's user, so when a
non-admin co-editor stored, it removed an admin's legitimately-authored embed
too (data loss). Now: toggle OFF still strips all (feature disabled); toggle ON
+ non-admin storer strips only NEWLY-introduced embeds and preserves those
already present in the persisted content (admin-vetted), via new helpers
collectHtmlEmbedSources + stripDisallowedHtmlEmbedNodes (identity = attrs.source,
already-vetted HTML). The ydoc reflect is now guarded by a deep-equal check so
an unrelated non-admin edit that touches no new embed doesn't churn the doc.
A non-admin still cannot add a new embed. Documents the allow-list TOCTOU
(best-effort snapshot read outside the lock; converges on next store).
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>
Resolve conflicts at shared registration points by unioning both features
(footnotes + the already-merged html-embed / page-embed work):
- slash-menu/menu-items.ts, editor extensions.ts: keep both imports + configures
- collaboration.util.ts: register footnote nodes and pageEmbed
- editor-ext marked.utils.ts: register footnote + html-embed markdown extensions
- editor-ext package.json/tsconfig.json/vitest.config.ts: union of test config
(jsdom env for footnote DOM tests + combined test/spec include glob)
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>
stripHtmlEmbedNodes only filtered children, so a (never-in-practice) bare
htmlEmbed root node would be returned as-is. Add a defensive root check that
returns an embed-free doc, making the helper total — it can never return a node
for which hasHtmlEmbedNode is true. Adds a unit test for the root case.
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>
The 'used in N pages' reverse-navigation method had zero callers in the merged
PR #17 — unreachable, untested code. Remove it. The reverse-navigation feature
can be (re)added with the method if/when it's actually built.
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>
PageTemplateController (added on this branch) guards its lookup/toggle routes
with UserThrottlerGuard, which depends on the throttler options provided by
ThrottleModule. CollaborationModule -> TransclusionModule registers that
controller, and the collab server bootstraps CollabAppModule, which did not
import ThrottleModule. The API server's AppModule does, so :3000 booted, but
the collab server (:3001) crashed at startup with
'Nest can't resolve dependencies of the UserThrottlerGuard ... THROTTLER:MODULE_OPTIONS'.
Without collab the editor can't sync, so live editing was broken on this branch.
Import ThrottleModule into CollabAppModule, mirroring AppModule, so the guard
resolves in the collab process too.
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>
Port two refinements from the GLM variant onto the Claude base:
- prepareAgentStep: add a comment note that AI SDK v7 renames the per-step
`system` field to `instructions` (v6 ^6.0.134 still uses `system`), so it
gets updated correctly on the next SDK bump.
- ai-chat.service.spec: add an explicit off-by-one boundary test for
prepareAgentStep, expressed via MAX_AGENT_STEPS instead of a hardcoded 18/19
so it tracks the constant if the cap changes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- extract collectAllIds / collectBranchIds into tree/utils and use them in
space-tree.tsx instead of inline closures
- drop the duplicate SidebarPageTreeDto, reuse the existing SidebarPageDto
for the /pages/tree endpoint
- type the getSpaceTree client call as api.post<{ items: IPage[] }>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
