UI opt-in for git-sync, mirroring the existing sharing/comments settings pattern
(no new endpoint, no new mechanism; orchestrator read query untouched):
- UpdateSpaceDto.gitSyncEnabled?: boolean.
- SpaceRepo.updateGitSyncSettings: jsonb-merge into settings.gitSync.<key>
(COALESCE || jsonb_build_object — never clobbers sibling sharing/comments);
stored as a real jsonb boolean so the orchestrator's
settings->'gitSync'->>'enabled' = 'true' matches.
- SpaceService.updateSpace handles the flag (audit diff) via the existing
CASL-guarded space update path (Manage/Settings).
- client: Switch in edit-space-form (optimistic mutate + revert-on-error,
readOnly-aware) + space types + 2 i18n keys.
- space.service.spec extended (calls updateGitSyncSettings; no-op when undefined).
tsc clean (server+client); jest src/core/space 4 pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fixes found by the live pull/push e2e:
- CRITICAL: driveCycle never checked out the 'docmost' branch before
applyPullActions, so Docmost content was written straight onto 'main',
clobbering local file edits before push could diff them. Now checkout
'docmost' before pull (applyPullActions commits there then checks out main +
merges) — mirrors the engine's pull main(). Round-trip now works both ways.
- add an unresolved-merge guard (SPEC §9): skip the cycle if the vault is
mid-merge instead of failing on checkout.
- SAFETY: enabledSpaces() is now STRICT opt-in — only spaces with
settings.gitSync.enabled===true; removed the all-spaces fallback that synced
every space (incl. a 92-page one) the moment GIT_SYNC_ENABLED flipped.
- SAFETY: per-cycle delete cap (GIT_SYNC_MAX_DELETES_PER_CYCLE, default 5):
dry-run the push, and if planned deletes exceed the cap, run the apply with
deletePage neutralized — phantom absence-deletions from a non-convergent vault
can't soft-delete real pages. Fails safe if the dry-run throws.
- fix manual trigger: TriggerGitSyncDto.spaceId needs @IsUUID or the global
whitelist ValidationPipe strips it (arrived undefined -> vault 'undefined').
Live-verified on an isolated flagged space: push (vault file edit -> Docmost
content, stamped lastUpdatedSource='git-sync') and pull (Docmost rename -> vault
file + meta) both work; an unrelated 92-page space stayed untouched throughout.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Native data plane for git-sync (plan §3, §8.1):
- provenance: widen actor to 'user'|'agent'|'git-sync' (jwt-payload,
auth-provenance decorator); PersistenceExtension resolves lastUpdatedSource
with precedence agent > git-sync > user, debounced history (like a human edit,
not the agent's immediate snapshot).
- GitmostDataSourceService implements @docmost/git-sync's GitSyncClient natively:
reads via PageRepo/SpaceRepo (listSpaceTree complete:true, getPageJson), writes
via PageService (create/removePage soft-delete/movePage with computed fractional
position/update-rename/restore) + the writeBody linchpin through collab
openDirectConnection('page.'+id, {actor:'git-sync'}) mirroring
collaboration.handler withYdocConnection 'replace'. bind({workspaceId,userId})
returns the context-bound client for the orchestrator.
- 10 unit/contract tests (mapping + soft-delete + move-position), tsc clean.
Known gap (closed in A.4b): PageService.create/update/movePage only branch on
actor==='agent'; git-sync provenance is already passed through so the row source
marker propagates once PageService honors 'git-sync'. Module/orchestrator/config
come next.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make @docmost/git-sync natively consumable by the CommonJS server (and jest):
build to CommonJS (tsconfig module CommonJS, drop type:module, strip .js from
relative imports), and lazy-load the only ESM-only dep (marked) via the dynamic
Function('import()') trick (mirrors docmost-client.loader.ts) with a require()
fallback so vitest's evaluator works too. git-sync tests stay green (314 pass,
3 expected fail).
Add the §13.1 idempotency gate (apps/server .../git-sync-converter-gate.spec.ts):
13 editor-ext docs (paragraphs/headings, marks, links, bullet/ordered/task lists,
blockquote, callouts, code block, hr, table, nested mix) round-trip
content(editor-ext) -> convertProseMirrorToMarkdown -> markdownToProseMirror ->
TiptapTransformer.toYdoc/fromYdoc(tiptapExtensions) -> canonicalize and assert
docsCanonicallyEqual. All green => the vendored converter's docmost-schema is
schema-compatible with editor-ext (no node/mark/attr loss), which the plan §13.1
requires before Phase B. The one intrinsic markdown-image lossiness (width/height
/align can't ride plain ) is isolated in a KNOWN DIVERGENCE block, not
hidden. Server tsc clean.
Co-Authored-By: Claude Opus 4.8 <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 fetches only from an
http(s):// base URL read at runtime from AI_AGENT_ROLES_CATALOG_URL; an empty or
non-http value yields a 502 (catalog unavailable). The image ships a per-branch
default for that URL (set in CI), still overridable at runtime via the env var.
- 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
reflects the per-branch build-time default that is runtime-overridable.
- Dockerfile: add ARG+ENV AI_AGENT_ROLES_CATALOG_URL in the installer stage as
the image default.
- CI: develop.yml builds with the develop raw URL; release.yml defines the main
raw URL once in workflow env and references it from both build steps.
- 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>
Address PR #227 re-review (comment 2193).
- Stability: `updatePageId`/`updateAlias` now `executeTakeFirstOrThrow`, so a row
reaped by a concurrent `removeAlias` between the read and the UPDATE (READ
COMMITTED) raises `NoResultError` instead of returning `undefined`. The service
maps that to a retryable `ConflictException` (`ALIAS_PAGE_RACE`) rather than a
200-without-alias (swap) or a generic 400 from `undefined.id` (rename). Tests
cover both branches.
- Simplification: drop the redundant secondary "unexpected unique index" warn and
the now-unused `UNIQUE_ALIAS_INDEX` const (the constraint name is already logged
unconditionally; both index branches still distinguish "Alias already taken" vs
ALIAS_PAGE_RACE).
- Architecture: extract `isUniqueViolation`/`violatedConstraint` into
database/utils.ts; adopt them in the share-alias service and favorite.repo
(the bare `23505` check). ai-agent-roles (#222) is on a separate unmerged branch
and should adopt them after #227 merges (noted at the helpers). Helper unit test
added.
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>
Addresses review on PR #227.
- setAlias confirmed-reassign branch: DELETE the target page's existing
alias row(s) BEFORE retargeting `byName` onto the page, instead of after.
The new partial unique index `(workspace_id, page_id)` is non-deferrable
and checked at each statement, so retargeting first momentarily left two
rows for the page -> immediate 23505 -> rolled-back tx surfaced as a
misleading "Alias already taken" (regressing a previously-working swap onto
a page that already had its own alias). The reordered branch needs no
trailing self-heal. JSDoc updated to describe the real ordering.
- catch block: the postgres@3.x driver exposes the violated index as
`err.constraint_name` (with `.constraint` as a fallback). Map
`share_aliases_workspace_id_alias_unique` -> "Alias already taken" and the
new `share_aliases_workspace_id_page_id_unique` -> a distinct ALIAS_PAGE_RACE
outcome (a concurrent same-page write, not a name clash). Always log the
constraint name on any 23505 so the race is diagnosable.
- migration 20260627T120000: document that the dedup DELETE is intended,
irreversible data loss (old duplicate `/l/<old>` links start 404ing after
upgrade; `down()` cannot restore the rows). Same note added to CHANGELOG
[Unreleased] Fixed.
Tests:
- integration: confirmed reassign onto a page that ALREADY has its own alias
(RED before the reorder); migration up() dedup scoping across pages and a
second workspace; mid-transaction error -> BadRequest with clean rollback.
- unit: constraint_name distinguishing (alias index, page_id index, fallback
`.constraint`, no-info default) and non-unique error -> BadRequest; retarget
test now asserts delete-before-update order.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Editing an existing share alias (e.g. slug `te` -> `ted`) failed to update
the displayed `/l/<alias>` link: `setAlias()` looked the requested slug up by
name and, if free, INSERTed a brand-new row, leaving the page with multiple
alias rows. The modal then read via `findByPageId().executeTakeFirst()` with no
`ORDER BY`, so Postgres returned an arbitrary (in practice the oldest, stale)
row. Every edit also spawned an orphan row that kept a live `/l/<old>` link
forever. Regression of #205.
Enforce the invariant "a page has EXACTLY ONE custom address":
- `setAlias()` now resolves the page's current alias row and RENAMES it in
place when the requested name is free (insert only when the page has none),
keeps the same-name no-op and the cross-page 409 `ALIAS_REASSIGN_REQUIRED`
+ confirmed-retarget flow, and after any successful write DELETEs all other
alias rows for the page (self-heal). Runs in one transaction so the page is
never transiently empty or duplicated.
- repo: add `updateAlias` (rename) and `deleteOthersForPage`; make
`findByPageId` deterministic with `ORDER BY created_at DESC, id DESC`.
- migration: dedup existing rows (keep newest per page) + a PARTIAL unique
index `(workspace_id, page_id) WHERE page_id IS NOT NULL` so dangling
aliases still coexist while live ones are one-per-page.
Co-Authored-By: Claude Opus 4.8 (1M context) <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>
Address review comment 2159 on the temporary-notes UI work.
Tests:
- tree-model: cover handleCreate's race-guard temporaryExpiresAt patch — (a)
server node inserted WITHOUT a deadline + create response carries one => node
gains the deadline; (b) node already has a deadline => not overwritten, prev
returned by reference.
- ws-tree.service.spec: broadcastPageCreated now asserts the deadline is carried
when present and pinned to null (`?? null`) when absent.
- page-embed-query (new spec): syncTemporaryExpiresInCache patches the in-tree
node's temporaryExpiresAt, and leaves the atom value at the same reference when
the id is absent from the loaded tree (no write).
Refactor (closes the drift bug-class at the root):
- Client: extract one canonical pageToTreeNode(page, overrides) mapper in
tree/utils and route buildTree, handleCreate's optimistic insert, the restore
mutation and the duplicate handler through it. Restore stays permanent (server
nulls temporaryExpiresAt) and duplicate stays permanent (server arms no timer)
— both now reflect the server without a reload, where before they dropped the
field entirely.
- Server: extract one toTreeNodeSnapshot(page) helper called by both the
PAGE_CREATED event enrichment (page.repo) and the addTreeNode broadcast
(ws-tree.service), so the optional temporaryExpiresAt can't drift between the
two literals.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Issue 1 — the sidebar tree's temporary-note clock marker did not appear/
disappear until a page reload when a note's temporary state changed.
- Make/unmake permanent from the page header menu and the in-page banner went
through syncTemporaryExpiresInCache(), which patched the page query cache but
never touched treeDataAtom, so the sidebar node kept its stale
temporaryExpiresAt. Patch the tree node there too (via jotai's default store),
so the marker updates without a reload.
- Creating a note as temporary showed no marker until reload: the create flow's
cache write (invalidateOnCreatePage) omitted temporaryExpiresAt, so the tree
rebuild (buildTree -> mergeRootTrees) overwrote the optimistic/socket node's
marker with undefined. Carry temporaryExpiresAt in that cached entry.
- Thread temporaryExpiresAt through the server addTreeNode broadcast (PAGE_CREATED
snapshot -> TreeNodeSnapshot -> broadcastPageCreated) so OTHER clients watching
the space also render the marker immediately, and harden handleCreate's
idempotency guard to patch the deadline if the broadcast won the insert race.
Issue 2 — the home and space-overview "New note" / "New temporary note" buttons
sat side-by-side and the temporary label clipped on narrow mobile widths. Lay
them out full-width, stacked vertically, and tint the temporary button orange
(matching the clock marker + banner) while the regular one stays neutral gray.
Tests: extend tree-socket-reducers.test.ts (addTreeNode carries
temporaryExpiresAt). Verified live with Playwright: marker appears on create and
toggles both ways with no reload; mobile buttons are stacked, full-width,
unclipped, and differently colored.
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>
develop merged 20260626T130000-share-aliases; rename this PR's migration to
20260626T140000 so the two no longer share a timestamp prefix.
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 deletion guard skips a note when its re-read deadline is still in the
future (user disarmed-then-re-armed in the race window between the batch
SELECT and the per-row re-read). The default stub returns an epoch deadline
(always < now), so the existing race tests never exercised the
`new Date(temporaryExpiresAt) >= now` branch; a regression dropping it or
inverting the comparison would pass unnoticed. Add a test that re-reads a
fresh future deadline and asserts removePage is not called.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Must-fix:
- CHANGELOG: add [Unreleased]/Added entry for temporary notes (#201).
- temporary-note-cleanup: re-check temporary_expires_at at deletion time so a
concurrent "Make permanent" (sets it NULL) between the batch SELECT and the
per-row removePage wins the race and the note is not trashed. Add unit tests
for the make-permanent and already-trashed race windows.
Non-blocking review items:
- temporary-note-cleanup: cap the sweep batch (LIMIT 500) so a large backlog is
not loaded into memory; remainder drains on the next hourly run.
- client: extract duplicated post-toggle cache sync into
syncTemporaryExpiresInCache() shared by the header menu and the banner.
- Remove the tautological migration spec that mocked the whole Kysely builder.
- Tests: cover create() frozen temporaryExpiresAt (workspace override + NULL
default fallback + non-temporary skips lookup) and restorePage disarming the
timer (temporaryExpiresAt: null).
Deferred (forward-looking, non-blocking): extract
PageService.computeTemporaryExpiresAt() to dedupe the deadline formula and drop
the @InjectKysely from PageTemplateController; replace migration unit test with
a real Postgres up/down integration test.
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>
"Temporary notes" with a death timer: created via a dedicated hourglass button
in the space-tree header, a note auto-moves to Trash after a configurable X
hours (default 24) unless explicitly made permanent ("structure or die").
Reuses existing mechanisms, mirroring is_template and the trash-cleanup job:
- New nullable column pages.temporary_expires_at (NULL = permanent; non-NULL =
frozen deadline) + partial index for the sweep; workspace column
temporary_note_hours (default via DEFAULT_TEMPORARY_NOTE_HOURS = 24).
- create-page DTO `temporary` flag; the deadline is frozen at creation so later
setting changes never reschedule existing notes.
- POST /pages/toggle-temporary (mirror of toggle-template): arm/clear the timer,
CASL-guarded via validateCanEdit, cross-workspace NotFound defense-in-depth.
- TemporaryNoteCleanupService: hourly @Interval sweep that soft-deletes expired
notes through the exact PageRepo.removePage path (recursive over children,
emits PAGE_SOFT_DELETED), attributed to the creator; idempotent via
deletedAt IS NULL filters.
- restorePage clears temporary_expires_at so a restored note can't be re-trashed.
- Workspace setting temporary_note_hours (audit-tracked) + a hours editor in
workspace General settings.
- Client: second create button, orange tree icon, tree + page-header menu toggle
("Make temporary"/"Make permanent"), an open-note banner with a rescue action,
and en/ru i18n.
Tests (unit): toggle-temporary controller (toggle/explicit/permission/cross-ws +
DTO validation), cleanup-job sweep (selection filters, per-note removePage,
error isolation), and a migration up/down sanity. Server tsc, client tsc -b,
and the page+workspace jest suites are green.
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>
The #205 share-aliases feature placed share-aliases.migration.spec.ts
inside src/database/migrations/. Kysely's FileMigrationProvider loads
EVERY file in that folder as a migration, so `migration:latest` imported
the test file and crashed with "ReferenceError: describe is not defined"
(no Jest globals under tsx). That broke the migration step shared by the
e2e-server, e2e-mcp and integration-test (test/test) jobs.
Move the spec one level up to src/database/ (matching the existing
src/database/jsonb-bind.spec.ts convention) so the migration runner no
longer sees it, and fix its relative imports
(./migrations/... and ./types/...). Jest still picks it up via the
src/**/*.spec.ts test glob. Verified locally: 3 passed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The controller's buildPageSlug truncates the page title via
`title?.substring(0, 70)` before slugifying, but no test drove that
branch (the only titled case was 16 chars). Add a resolvable-alias
case with a 119-char title whose 70-char boundary falls mid-word and
assert the 302 target's slug reflects only the first 70 characters.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code-review follow-ups (Approve-with-comments) for batch #197
(context badge #189 / e2e in CI #187 / inline MCP test #170):
- server: extract the duplicated chatContextWindow ::text->positive-int
coercion (resolve() + getMasked()) into an exported parsePositiveInt
helper and unit-test its branches (200000/1.9/0/-5/""/abc/undefined),
closing the untested read-path gap.
- client: merge the two backward scans over messageRows into one pure,
exported selectContextBadge helper (numerator and denominator still
taken from the most recent row carrying EACH value) and unit-test the
different-rows and fresh-zero-doesn't-shadow cases.
- client: extract the MCP "Test" button tristate presentation into a pure
mcpTestButtonView helper (collapses the two parallel if/else chains) and
unit-test idle/ok-with-tools/ok-no-tools/failed label+tooltip branches.
- ci: redirect the backgrounded prod server's stdout/stderr to a log file
in e2e-mcp and cat it on failure, so a start-up crash is diagnosable
instead of surfacing only as the generic health timeout.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the two blocking test-coverage specs requested in the PR #214 review and
clear the cheap non-blocking items.
Must-fix:
- share-alias-redirect.controller.spec.ts: routing/leak guard for the public
GET /l/:alias resolver (modeled on share-seo.controller.routing.spec). Pins
302-to-canonical on a hit; SPA index without a 302 for unknown/dangling/
unreadable aliases and a null workspace (no name-existence leak); defensive
percent-decoding treated as unknown; self-hosted findFirst vs subdomain
findByHostname workspace resolution; 404 when no built client index exists.
- share-alias.controller.spec.ts: authz gates with mocked PageRepo/ShareService/
ShareAliasService/PageAccessService. Covers cross-workspace/nonexistent page
-> NotFoundException, validateCanEdit, resolveReadableSharePage null ->
BadRequestException, isSharingAllowed false -> ForbiddenException, set happy
path delegation, remove() of a dangling alias (pageId null) skipping
validateCanEdit but still deleting, and for-page validateCanView.
Cheap review items:
- Remove dead Logger import/field from ShareAliasRedirectController.
- Remove dead PagePermissionRepo import/dependency from ShareAliasController.
- Register the new share-alias UI strings in en-US and ru-RU catalogs.
- Add an [Unreleased]/Added CHANGELOG entry for /l/:alias (#205).
- Drop the tautological boilerplate assertions from the migration spec
(exports up/down; runtime checks of typed entity literals).
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>
Add a retargetable, human-readable vanity link namespace /l/<alias> that
sits alongside the untouched /share/... routes.
- New share_aliases table (workspace-scoped, UNIQUE(workspace_id, alias),
page_id nullable ON DELETE SET NULL so the address outlives its target).
- ShareAliasRepo + ShareAliasService (create / no-op / 409 reassign guard /
availability / request-time readable-target resolution through the single
existing share boundary).
- Public ShareAliasRedirectController (GET /l/:alias) issues a 302 (never 301,
the target is mutable) to the canonical /share/:key/p/:slug page; unknown /
dangling / no-longer-readable aliases serve the SPA index with no leak.
'l/:alias' excluded from the global /api prefix.
- Authenticated ShareAliasController (set/remove/availability/for-page).
- Shared ASCII-only normalize/validate util (server + client copies).
- Client: Custom address block in the share modal (live normalize + debounced
availability + copy + reassign confirmation dialog).
- Unit tests: util, repo SQL-shape, service semantics, migration/entity sanity
(server jest) + client alias util (vitest).
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>
persist-1: onStoreDocument wrapped the page write in a try/catch that only
logged and swallowed the error, then resolved "successfully". hocuspocus
destroys/unloads the in-memory Y.Doc right after the hook resolves (the only
copy of the latest edit), so a transient DB error (deadlock, serialization
failure, dropped connection) silently lost the edit. Worse, the post-store
branch ran on the partially-assigned `page`, broadcasting a phantom
"page.updated" and enqueueing a history snapshot for content never written.
Wrap the write in a small bounded retry (3 attempts) so the save is
re-attempted while we still hold the doc, and clear `page` on failure so the
success-only side effects never report a save that didn't happen.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
attach-1: when the same attachmentId was referenced by more than one page
in a duplicated subtree, the per-attachmentId map held only a single copy
entry, so the last page processed clobbered the others. The downstream
ownership guard (`attachment.pageId !== oldPageId`) then matched at most one
page and skipped the lone DB row entirely: no blob copied, no new row, every
copy's image 404'd. Key the map to a list of entries and copy one blob/row
per referencing page; drop the now-incorrect ownership guard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getPageBreadCrumbs (ancestor CTE) and forceDelete (descendant CTE) used
withRecursive + unionAll with no CYCLE clause or depth cap. If a parent/child
cycle already exists in the data (e.g. one slipped in via the #7 TOCTOU race),
both queries loop forever — hang / statement timeout. Worse, the move guard
itself runs the ancestor CTE, so a cycle would disable the very guard meant to
prevent it (#207#8).
Add a depth counter bounded by MAX_PAGE_TREE_DEPTH to both recursive CTEs; the
walk stops at the cap, so a cycle yields a bounded result instead of hanging.
Real page trees are only a few levels deep, so the cap never truncates a
legitimate result. getPageBreadCrumbs selects an explicit column list (not
selectAll) so the internal depth counter never leaks into the breadcrumb shape.
Adds an integration test that seeds an A<->B cycle directly and asserts both
getPageBreadCrumbs and forceDelete return bounded / complete under a short
connection-level statement_timeout instead of hanging.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The server-side move cycle guard (getPageBreadCrumbs) and the move UPDATE ran
as two separate, unlocked statements. Two concurrent moves ("A under B" and
"B under A") could each read the same pre-write acyclic snapshot, both pass the
guard, then persist A.parentPageId=B AND B.parentPageId=A — a parent/child
cycle (TOCTOU, #207#7).
Run the cycle check and the UPDATE inside one transaction (executeTx) guarded
by a per-space advisory lock (pg_advisory_xact_lock, held until COMMIT) so all
moves within a space serialize: the second mover blocks until the first commits
and then sees the freshly written parent, so its guard rejects the cycle.
getPageBreadCrumbs gains an optional trx so the check runs on the locked snapshot.
Adds an integration test driving two opposing concurrent movePage calls and
asserting no cycle ever persists and exactly one move is rejected. Updates the
movePage unit-test stubs for the new transactional path.
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>
