Implements all reviewer comments (code-review, red-team, and test-strategy
audit), accepting the recommended variants.
Server — realtime service (ai-realtime.service.ts):
- SSRF: pin the validated IP via a WebSocket `lookup` hook that re-checks every
resolved address with isIpAllowed (mirrors external-mcp buildPinnedDispatcher),
closing the TOCTOU/DNS-rebinding window; fix the misleading comment.
- no-silent-loss: on Stop, drain the in-flight segment (bounded 2.5s) and deliver
the final via onFinal before closing instead of dropping the tail.
- fail-closed deriveRealtimeUrl: a non-empty unparseable base now THROWS (no
silent api.openai.com fallback that would leak a self-hosted key); http://ws://
bases rejected (plaintext key). Path normalization preserved.
- parseUpstreamEvent keys the accumulator by item_id+content_index so GA segments
don't concatenate.
- inject a wsFactory seam for testing; also fix a latent bug — `import WebSocket
from 'ws'` resolved to undefined at runtime (no esModuleInterop) -> import=require.
- unref idle/max/drain timers.
Server — realtime gateway (ai-realtime.gateway.ts, session-limits.ts):
- reject revoked/disabled users and inactive sessions (mirror jwt.strategy:
findById+isUserDisabled + findActiveById) with NO counter increment.
- CSWSH: Origin allowlist (matching APP_URL, or no Origin for native clients)
before auth, no increment.
- extract SessionCounters (delete-at-zero, never negative) + pure canConnect
(both caps >= checked before any increment); document the per-process/in-memory
cap caveat (single-replica only).
Client:
- dictation-group: realtime final now inserts at the captured rangeRef SNAPSHOT
(not the live caret) and guards editor.isEditable; single-space separator.
- use-realtime-dictation/realtime-dictation-client: stop-during-acquisition tears
down the mic (no leak / button reset); reconnect re-emits start (double-start
guarded); interim ghost cleared on teardown; io() options de-duplicated.
- pcm16-worklet: flush the partial sub-frame tail on stop; one-pole anti-aliasing
low-pass before 48k->24k.
- extract shared mic-capture (acquireMicStream/mapGetUserMediaError, used by batch
+ realtime), pure DSP (pcm16-dsp.ts), and the session reducer/baseLanguageSubtag;
extract applyInterimMeta/clampRange/resolveUrl/appendFinalToDraft.
Tests + infra: +~150 server tests (deriveRealtimeUrl, parseUpstreamEvent branches,
openSession/lifecycle/timers/testConnection via fake ws, gateway auth/caps/no-leak,
realtime-test admin contract, AiSettings update/resolve, DTO boolean, SSRF deny)
and +~140 client tests (DSP property/edge, resampler continuity, framing, reducer,
mic-capture, RealtimeDictationClient/MicButton, ProseMirror interim regression +
history guards, appendFinalToDraft, resolveKeyField, route contract). Added
@vitest/coverage-v8. CHANGELOG [Unreleased] entry incl. the single-replica caveat.
Review: APPROVE WITH SUGGESTIONS (no critical/regression); applied the drain-timer
unref. Server tsc clean + 358 tests; client tsc clean + 201 tests; vite build ok.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Layer an optional realtime speech-to-text path on top of the existing
batch dictation, so transcribed text appears as the user speaks.
Transport A2: browser <-> our server (Socket.IO `/ai-realtime`) <->
OpenAI Realtime (raw ws). The provider API key never leaves the server;
the upstream URL is SSRF-checked before connecting; the gateway enforces
the dictation+dictationRealtime gate, cookie-JWT auth and per-user/
per-workspace concurrency caps. Implemented against the GA (2026) OpenAI
Realtime transcription contract (session.update / audio.input.format /
server_vad), not the now-removed beta shape.
Editor UI B2: interim text is shown as a meta-only ProseMirror ghost
decoration (no Yjs/history noise); only completed segments are committed.
Chat shows interim as a dimmed tail. The mic button switches realtime vs
batch by the workspace flag; batch remains the default and fallback.
Server:
- AiRealtimeService (upstream ws proxy, normalized events, idle/max-
duration timeouts, idempotent teardown) + parseUpstreamEvent unit tests
- AiRealtimeGateway (Socket.IO `/ai-realtime`) wired into AiChatModule
- admin-gated POST /ai-chat/realtime/test connectivity probe
- config: settings.ai.dictationRealtime + provider sttRealtimeModel/
sttRealtimeBaseUrl (realtime key reuses sttApiKey; no new secret)
Client:
- pcm16 AudioWorklet (24kHz mono PCM16), RealtimeDictationClient,
use-realtime-dictation hook (status/start/stop/cancel + onInterim/onFinal)
- RealtimeMicButton + dictation-interim ProseMirror decoration
- editor/chat integration + AI settings UI (toggle, model, test endpoint)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fit full role-card description text in the AI chat empty state and show a
generic "AI is typing…" indicator (role name kept only as the dimmed
interlocutor label).
The typing indicator rendered "<role name> is typing…". Show a generic
"AI is typing…" instead and keep the role/identity name only in the
dimmed interlocutor label above the typing dots.
- typing line now always renders t("AI is typing…")
- add the "AI is typing…" key to en-US and ru-RU locales
- sync stale doc comments that referenced the old text
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The colored role cards in the AI chat empty state truncated their
admin-configured description with an ellipsis and could clip the top row
when the cards overflowed. Make the full text fit:
- drop the description lineClamp so the whole text renders
- add overflow-wrap: anywhere so long unbreakable tokens (URLs) wrap
- switch the cards container to align-content: flex-start so an
overflowing top row stays reachable while scrolling (the parent
Mantine Center still vertically centers the block when it fits)
- widen the card max-width 180px -> 200px for more text room
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-ups from the multi-aspect review of the e5bc82c7..d4658d4c range.
- CHANGELOG: document under [Unreleased] that the default per-workspace
hourly public-share assistant cap was lowered 300 -> 100 after the
v0.93.0 tag (#62). v0.93.0 shipped 300, so existing deployments that
never set SHARE_AI_WORKSPACE_MAX_PER_HOUR drop to 100 on upgrade.
- Recreate the still-open Section 3 (AiChatService.stream integration
coverage) of the deleted feature-test-coverage-deferred.md as a focused
backlog doc so the test debt stays tracked; Sections 1-2 are already
closed by the integration harness (PR #115).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rework the new-chat role-card empty state:
- Remove the "Universal assistant" card; universal assistant is now the
implicit default the user gets by typing without picking a card.
- Show each role's description on its card (under the emoji and name).
- Clicking a card immediately starts the chat: it binds the role to the
new chat and sends the default opening prompt "Take a look at the
current document" (one click, no separate select step). roleIdRef is
set synchronously before sendMessage so the create request carries the
role.
- Show the current role's name in the window header badge and as the
assistant's display name (transcript label + "… is typing…"), falling
back to "AI agent" for a role-less chat. selectChat resets the picked
role so it cannot leak into an unrelated existing chat.
- Add the "Take a look at the current document" i18n key (en-US, ru-RU).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pre-merge review follow-up for the parseNodeArg dedupe (PR #114):
- Restore docs/backlog/ai-chat-tool-definitions-duplicated.md instead of
deleting it: it still tracks open debt (unified spec registry + ProseMirror
<-> Markdown converter unification) that this branch defers, and
docs/git-sync-plan.md links to its converter section. Mark the node-arg
quirk as done and add a Progress section.
- Reword the in-app helper header from "byte-for-byte" to "behaviorally
identical": the two copies differ in comments/quote style; only the logic,
throw messages and branch order match.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ru-RU was missing most AI-chat keys, so the chat/typing widgets rendered
mixed-language (some keys fell back to en-US). Fill the full AI-chat string
set in ru-RU and document the maintenance policy.
- ru-RU/translation.json: add the 24 missing AI-chat keys (labels, typing
indicator, Ask-AI widget, public-share, error messages); keep the typing
keys grouped; existing translations untouched.
- i18n.ts: add a policy comment near fallbackLng — en-US is the source of
truth; en-US + ru-RU are fully maintained; the other 10 locales
intentionally rely on the en-US fallback until contributed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The fail-closed limiter behavior (#62 primary item) already shipped; this
finishes the issue by lowering the default hourly per-workspace cap from 300
to 100 to better fit real anonymous-assistant load. Still overridable via
SHARE_AI_WORKSPACE_MAX_PER_HOUR.
- public-share-workspace-limiter.ts: SHARE_AI_WORKSPACE_MAX_PER_WINDOW 300 -> 100.
- .env.example: documented default + example value 300 -> 100.
- public-share-chat.spec.ts: update the default-cap assertion to 100.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements Option 2 of #93. The restricted branch of broadcastPageMoved
previously resolved its audience twice — emitToAuthorizedUsers and
emitDeleteToUnauthorized each ran an independent fetchSockets +
getUserIdsWithPageAccess — leaving a race window between the two snapshots
where a socket could receive both the move and the delete (leak) or neither
(lost compensating delete).
- ws.service.ts: add emitMoveWithRestrictionSplit() that takes ONE socket
snapshot and ONE authorization resolution, then partitions the room:
authorized users get the moveTreeNode, everyone else (unauthorized +
anonymous) get the compensating deleteTreeNode. Disjoint + complete by
construction. Remove the now-unused emitToAuthorizedUsers /
emitDeleteToUnauthorized; keep private broadcastToAuthorizedUsers (still
used by emitRestrictedAwareToSpace).
- ws-tree.service.ts: broadcastPageMoved restricted branch now drives move +
delete from the single method.
- specs: assert the single method is used and that fetchSockets /
getUserIdsWithPageAccess are each called exactly once (single snapshot);
re-route ws-service.spec to emitTreeEvent after the method removal.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Builds the deferred integration tests from docs/backlog/feature-test-coverage-
deferred.md that needed real infra (a test Postgres + real Redis) which the repo
lacked. Runs against an isolated, auto-created docmost_test database and Redis
logical DB 15 — never the dev data.
Harness (apps/server/test/integration/, run via new `pnpm --filter server test:int`
=> jest --config test/jest-integration.json; default unit `jest` is untouched and
excludes these via the *.int-spec.ts name + rootDir):
- db.ts: buildTestDb() mirrors database.module.ts exactly (PostgresJSDialect,
CamelCasePlugin, bigint to:20/from:[20,1700] parsing) + minimal seed helpers.
- global-setup.ts: DROP/CREATE docmost_test, CREATE EXTENSION vector, migrate to
latest via Kysely Migrator (fails loud on any errored migration).
- global-teardown.ts: closes the pool.
Coverage (5 suites, 16 tests, all green against live PG+Redis):
- WorkspaceRepo.updateSetting: jsonb-merge persists htmlEmbed without clobbering
sibling ai/sharing namespaces (the kill-switch write half).
- AiAgentRoleRepo: soft-delete exclusion, cross-workspace tenant isolation,
duplicate (name,workspace) -> 23505, name reusable after softDelete (partial
unique index WHERE deleted_at IS NULL), same name across workspaces allowed.
- page_template_references: deleting either source or referenced page cascades
the link row (onDelete cascade) — real FK, not mocked.
- PublicShareWorkspaceLimiter vs REAL Redis: real ioredis EVAL of the sliding-
window Lua — max boundary (3 admit / 4th deny), re-admit after the window
slides, same-ms distinct members. Catches Lua bugs a FakeRedis cannot.
- AiChatRepo.findByCreator: role-badge join (enabled->badge; soft-deleted or
disabled role -> null).
Review: APPROVE; applied its two hardening suggestions (fail loud on errored
migration result even without a top-level error; TEST_REDIS_URL override + ping
preflight). tsc clean; unit run excludes int-spec (verified).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
First, safe step of docs/backlog/ai-chat-tool-definitions-duplicated.md: the
"node may be a JSON object OR a JSON string" quirk was hand-copied at 6 tool
sites. Extract it into a single parseNodeArg() helper per package and call it at
every site. Behavior-preserving — each site's throw message is byte-identical
(patch/insert: 'node was a string but not valid JSON'; update_page_json: 'content
was a string but not valid JSON'); no tool name/description/schema changed.
Two helper copies (packages/mcp/src/lib/parse-node-arg.ts and
apps/server/src/core/ai-chat/tools/parse-node-arg.ts) are intentional: the
ESM-only @docmost/mcp cannot be imported by the CommonJS server (it is loaded at
runtime via the Function('import()') trick), so runtime code cannot cross that
boundary by a normal import. Each copy is now the single source within its
package (6 inline copies -> 2 helpers). packages/mcp/build rebuilt in sync.
Tests: parse-node-arg.spec.ts (server, Jest) + parse-node-arg.test.mjs (mcp,
node:test) — object passthrough, valid-string parse, invalid-string throw with
the right message. Server tsc clean; mcp suite 254 pass; agent structural-edit
path verified live in-browser (agent inserted a node, persisted to the doc).
Deferred (documented for the record, since the backlog doc is removed with this
commit): the FULL transport-agnostic tool-spec registry (one name+schema+
description per tool shared by both transports) and deriving DocmostClientLike
from the real client type. Both are blocked by the current architecture, not by
effort: (1) @docmost/mcp ships no type declarations and is ESM-only, so a
type-only derivation needs declaration emission + tsconfig path wiring, and the
real client's precise return types break the in-app tool test stubs (attempted,
reverted to keep tsc green); (2) the two transports intentionally DIVERGE in tool
NAMES (snake_case x38 vs camelCase x41), membership (in-app adds getCurrentPage/
listSidebarPages, omits delete_comment/image tools) and model-facing
DESCRIPTIONS, so a unified registry would change behavior on BOTH the agent and
external MCP clients and needs its own verification pass. This is forward-looking
debt (the code is correct today), to be done incrementally.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the new-chat <Select label="Agent role"> picker with colored role
cards rendered as the empty-state of a brand-new chat (centered in the window),
per docs/backlog/ai-chat-role-cards-empty-state.md. Clicking a card selects that
identity; sending without a pick falls back to the Universal assistant; the
cards disappear once the chat is non-empty. Purely client-side — the existing
selectedAiRoleIdAtom + roleId request wiring (server role fixation on chat
creation) is unchanged.
- new RoleCards rendered through the existing emptyState prop chain
(AiChatWindow -> ChatThread -> MessageList); MessageList already supported it.
- Universal assistant card (gray, value null, default-selected) + one card per
enabled role, color cycled from a 10-name Mantine palette via the pure
roleCardColor() helper; theme-aware CSS vars (light/-light-color/-filled).
- each card is an UnstyledButton with aria-pressed for a11y + testability.
- tests: role-card-color (palette cycling, negative-safe) + role-cards.test.tsx
(render, emoji/name, selection highlight, click -> onSelect). 9 tests green,
client tsc clean.
Verified live in-browser: cards (not a Select) show for a new chat; selecting
Пират binds the chat to that role end-to-end (badge + pirate reply); no pick =>
Universal; cards vanish after the first message.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 'current page' feature (client useMatch openPage + server getCurrentPage
tool + system-prompt injection) was already implemented & merged; this backfills
its missing test coverage and removes the completed backlog doc.
- extract pure resolveCurrentPageResult(openedPage) into current-page.util.ts
(byte-identical to the prior inline getCurrentPage tool body) so it is
unit-testable without the dynamically-imported ESM Docmost client; the tool
now delegates to it.
- current-page.util.spec.ts: 7 cases (null/undefined/no-id/empty-id/full/no-title).
- ai-chat.prompt.spec.ts: +8 cases for the openedPage context line (title+pageId
present, Untitled fallback for blank/whitespace title, no line when absent/blank
id, and sandwich ordering before the trailing safety block).
Verified live in-browser: client sends openPage{id,title} on a page and null
off-page; the agent invokes getCurrentPage and answers with the real title+id.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The feature is already implemented and merged into develop (f6e216cb):
auto-collapse the AI chat window into its header on outside-page pointer,
expand on header click, with keyboard a11y. Verified live in-browser and
covered by collapse-helpers.test.ts (9 tests). Removing the now-completed
planning doc.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract makeConnectHandler(queryClient) (owning the firstConnect flag) from
UserProvider and test it: first connect does NOT invalidate; a reconnect
invalidates both root-sidebar-pages + sidebar-pages. Behavior-identical (#66).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ru-RU had only '{{name}} is typing…' but not 'AI agent' / 'AI agent is typing…',
so the Russian typing indicator was mixed-language. Add them (AI-агент / AI-агент
печатает…) grouped with the named key. en-US is already complete; other locales
intentionally keep the en-US fallback (full translation is a separate effort).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Export + test isHtmlEmbedFeatureEnabled: the 'HTML embed' slash item is hidden by
default / when the toggle is off / on broken localStorage (no throw), shown only
when the workspace toggle is exactly true.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract parse/renderHtmlEmbedHeight and test: '300'->300, absent->null,
'abc'->null (pins the NaN guard), '120px'->120; render 120->data-height, null/0->{}.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract clampHeight + isTrustedHeightMessage + the HTML_EMBED_SANDBOX token
constant from the NodeView and test them: clamp bounds; reject a resize message
from a foreign window / wrong type / NaN/Infinity; accept a valid same-source
finite message; assert the sandbox is exactly 'allow-scripts allow-popups
allow-forms' (no allow-same-origin) and rendered via srcDoc (not src).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add htmlEmbed to the schema toYdoc/fromYdoc acceptance cases, asserting source +
height survive, so removing the passthrough node (which prevents 'Unknown node
type: htmlEmbed' on MCP/AI edits of an embed page) fails CI.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DTO: trackerHead @IsString/@MaxLength(20000) + htmlEmbed @IsBoolean accept/reject
cases. CASL: a non-admin updating trackerHead/htmlEmbed gets ForbiddenException
(update not called); owner/admin proceed. Audit: a no-op trackerHead re-save
doesn't enter the audit diff.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the admin trackerHead <head> injection into a pure injectTrackerHead()
and test it: a snippet containing $&/$$/backtick-dollar survives BYTE-FOR-BYTE
(pins the function-replacer fix), empty/whitespace/undefined and a missing </head>
leave the html unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the shared assistant-name predicate (resolveAssistantName: trimmed name
or null) used by typing-indicator + message-item, and unit-test the branches
(name shown; whitespace-only -> 'AI agent' fallback; undefined -> fallback).
Behavior-identical (|| -> ?? since the helper returns null).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Relocate resolveTrustProxy from main.ts (untestable — bootstraps on import) to
integrations/environment/trust-proxy.util.ts and import it back. Unit-test every
branch (empty/undefined -> safe loopback/private default; true/false; hop count;
trim; CIDR/negative passthrough) so a regression can't silently re-open the XFF
spoofing hole (#61).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Covers the #63 guard: a message with a non-text part -> 400 'Unsupported message
content'; a message mixing text + a non-text part still 400s (before the 413
size check).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds the missing tests for the #67 guard: self-move and a destination inside the
moved page's subtree both throw BadRequestException before updatePage; a
legitimate move proceeds. Mocks pageRepo + spies getPageBreadCrumbs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- 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>
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>
The anonymous public-share "Ask AI" chat labeled every assistant turn
with the generic "AI agent" even when an Assistant identity (agent role)
was configured. Surface the configured identity name instead, falling
back to "AI agent" when no identity is set.
- server: AiSettingsService.resolvePublicShareAssistantName resolves the
configured role's name (null when unset/missing/disabled), mirroring
PublicShareChatService.resolveShareRole; ShareController returns it as
aiAssistantName on /shares/page-info (only when the assistant is on).
- client: thread aiAssistantName -> ShareAiWidget -> MessageList ->
MessageItem/TypingIndicator via an optional assistantName prop; the
internal chat omits it and keeps showing "AI agent".
- i18n: add "{{name}} is typing…" (en-US, ru-RU) for the typing line.
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.
isBlocked was checked synchronously but recordFailure ran only AFTER the bcrypt
awaits, so N concurrent /mcp Basic requests for one email all slipped past the
threshold. Add FailedLoginLimiter.tryReserve (atomic synchronous check+increment)
+ release (undo), and reserve all 3 keys BEFORE any await so the (threshold+1)-th
concurrent attempt is rejected before its bcrypt runs. The reservation IS the
failure record (post-await recordFailure removed -> counted exactly once). Non-
credential early throws (missing workspace, SSO/MFA gate) and business errors
release the reservation so they don't burn a victim's budget; success clears.
Tests prove login() runs exactly threshold times under concurrency and that
gate/config rejects don't consume budget.
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>
