fix(share): custom address edit renames in place instead of duplicating (#226) #227

Ghost · 2026-06-27T01:03:07+03:00

Ghost commented

2026-06-27 01:03:07 +03:00

Закрывает #226 (регрессия #205).

Причина

Редактирование кастомного адреса (te→ted) делало INSERT новой строки-алиаса вместо переименования (уникальность была только по (workspace_id, alias), не по page_id), а findByPageId().executeTakeFirst() без ORDER BY отдавал произвольную (старую) строку → ссылка не обновлялась + копились осиротевшие живые /l/<старое>.

Фикс

setAlias() — инвариант «ровно один адрес на страницу»: свободное имя + у страницы уже есть алиас → переименование на месте; нет → INSERT; то же имя → no-op; занято другой страницей → прежняя 409 ALIAS_REASSIGN_REQUIRED; после успеха → deleteOthersForPage (самолечение). В транзакции.
repo: updateAlias, deleteOthersForPage; findByPageId детерминирован (ORDER BY createdAt DESC, id DESC).
Миграция 20260627T120000-share-aliases-one-per-page — дедуп существующих строк (оставляет новейшую) + частичный уникальный индекс (workspace_id, page_id) WHERE page_id IS NOT NULL (dangling-алиасы с NULL сосуществуют). Reversible.

Проверка

56 unit + 6 integration (живой pg, миграция применяется чисто) зелёные; tsc + nest build ок.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

🤖 Generated with Claude Code

Закрывает #226 (регрессия #205). ## Причина Редактирование кастомного адреса (`te`→`ted`) делало INSERT новой строки-алиаса вместо переименования (уникальность была только по `(workspace_id, alias)`, не по `page_id`), а `findByPageId().executeTakeFirst()` без `ORDER BY` отдавал произвольную (старую) строку → ссылка не обновлялась + копились осиротевшие живые `/l/<старое>`. ## Фикс - `setAlias()` — инвариант «ровно один адрес на страницу»: свободное имя + у страницы уже есть алиас → **переименование на месте**; нет → INSERT; то же имя → no-op; занято другой страницей → прежняя 409 `ALIAS_REASSIGN_REQUIRED`; после успеха → `deleteOthersForPage` (самолечение). В транзакции. - repo: `updateAlias`, `deleteOthersForPage`; `findByPageId` детерминирован (`ORDER BY createdAt DESC, id DESC`). - Миграция `20260627T120000-share-aliases-one-per-page` — дедуп существующих строк (оставляет новейшую) + **частичный уникальный индекс** `(workspace_id, page_id) WHERE page_id IS NOT NULL` (dangling-алиасы с NULL сосуществуют). Reversible. ## Проверка 56 unit + 6 integration (живой pg, миграция применяется чисто) зелёные; tsc + `nest build` ок. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Ghost added 1 commit 2026-06-27 01:03:07 +03:00

fix(share): keep exactly one custom address per page on alias edit (#226 ) c18abf84c6

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>

vvzvlad commented

2026-06-27 02:31:31 +03:00

Code review — multi-aspect (security · stability · conventions · docs · regressions · test-coverage · simplification · architecture)

Вердикт: Request changes. Ревью на реальном коде ветки против develop (merge-base e9409e24), 6 файлов, +584/−53. Сам фикс корректен для основных сценариев (rename te→ted на месте, self-heal, детерминированный findByPageId, миграция с дедупом и частичным уникальным индексом). Но есть один блокирующий баг корректности: подтверждённый перенос адреса (confirmReassign) на страницу, у которой уже есть собственный алиас, теперь падает из-за порядка операций относительно нового уникального индекса. Критичных проблем (outage / потеря данных / эксплойт) нет — поэтому Request changes, а не Block merge. Security и Conventions — чисто (LGTM).

⛔ Must fix before merge

[stability/regressions] Переставьте удаление перед updatePageId в ветке подтверждённого reassign — share-alias.service.ts:96-108

В ветке «имя занято другой страницей + confirmReassign» код сначала делает updatePageId(byName.id, pageId) и только потом deleteOthersForPage. Новая миграция 20260627T120000 добавляет обычный (non-deferrable) частичный уникальный индекс (workspace_id, page_id) WHERE page_id IS NOT NULL, который Postgres проверяет немедленно по завершении каждого statement (уникальный индекс нельзя сделать DEFERRABLE).

Реалистичный сценарий: у страницы A адрес shared, у страницы B уже есть свой адрес bee; пользователь подтверждает перенос shared на B. На шаге updatePageId появляются две строки с page_id = B → немедленный 23505 до того, как отработает deleteOthersForPage. Транзакция откатывается, а catch на строках 152-153 переводит это в вводящую в заблуждение ошибку ConflictException('Alias already taken') — хотя имя как раз было свободно занять. До этого PR ограничения (workspace_id, page_id) не было и такой swap работал → это регрессия ранее работавшего сценария.

Fix: в этой ветке сначала вызвать deleteOthersForPage(pageId, byName.id, workspaceId, trx) (удалит существующий алиас целевой страницы; byName.id пока указывает на старую страницу, поэтому исключение по keepId безвредно), затем updatePageId(byName.id, pageId, workspaceId, trx) — после этого byName.id остаётся единственной строкой для pageId, завершающий deleteOthersForPage уже не нужен. Заодно это делает истинным JSDoc на строках 50-53 («…the page never transiently has zero or duplicate rows»), который сейчас противоречит фактическому порядку в этой ветке (та же неточность продублирована в теле коммита). Добавьте интеграционный тест на этот случай (см. «Test coverage»).

[warning][stability] Различайте, какой именно индекс дал 23505 — share-alias.service.ts:144-157
Catch трактует любой 23505 как «имя уже занято». После добавления второго уникального индекса (workspace_id, page_id) это уже не всегда так (гонка двух setAlias по одной странице, да и баг выше). Fix: анализировать err.constraint/err.detail — маппить share_aliases_workspace_id_alias_unique → «Alias already taken», а share_aliases_workspace_id_page_id_unique → отдельный код/сообщение; как минимум логировать err.constraint.
[suggestion][regressions] Опишите в release notes, что дедуп-миграция гасит старые /l/<old> ссылки — 20260627T120000-share-aliases-one-per-page.ts:19-26
DELETE в up() необратимо удаляет все строки страницы кроме новейшей. Старые слаги резолвятся публичным редиректом через findByAliasAndWorkspace (не findByPageId), так что ранее живые /l/<старое-имя> после апгрейда начнут отдавать SPA-404. Это и есть суть фикса (гасятся ровно дубликаты от старого бага), но удаление тихое и необратимое (down() восстанавливает индекс, но не строки). Изменений в коде не требуется — только пометка в описании/чейнджлоге.

Test coverage

Покрытие в целом хорошее (56 unit + 6 integration), но есть пробелы:

(связан с must-fix) Нет теста на подтверждённый reassign, когда у целевой страницы УЖЕ есть свой алиас — share-alias-one-per-page.int-spec.ts:181-214. Текущий «cross-page collision» переносит имя на страницу B без собственного алиаса, поэтому баг выше не ловится. Добавьте: A←shared, B←bee, затем setAlias({pageId: B, alias: 'shared', confirmReassign: true}) → у B ровно одна строка shared, у A — ноль.
[suggestion] Дедуп-DELETE проверяется только на одной странице/одном workspace — share-alias-one-per-page.int-spec.ts:94-131. Скоупинг self-join по (workspace_id, page_id) (что DELETE не залезает в чужие страницы/воркспейсы) не проверен; к тому же SQL дублируется инлайном вместо вызова up() миграции. Засейте дубликаты для ≥2 страниц (и второго workspace).
[suggestion] Нет теста на rollback/общую ошибку транзакции — ветка fallthrough BadRequestException('Failed to set alias') (share-alias.service.ts:155-156) и откат при ошибке в середине транзакции не покрыты.

Architecture & design (forward-looking, non-blocking)

Инвариант «один алиас на страницу» проверяется БД-индексом в середине транзакции → сервис вынужден аккуратно упорядочивать statements. Это и есть корень must-fix-бага.
оставить частичный индекс, починить порядок в reassign-ветке* (effort: s). Минимально, сохраняет dangling-поведение. Это и есть рекомендованный must-fix.

Self-heal deleteOthersForPage вызывается в каждой из трёх ветвей записи — признак отсутствующего примитива «ровно один алиас на страницу».
свести deleteOthersForPage к одному вызову в конце setAlias* (effort: s): безопасно только вместе с фиксом порядка в reassign-ветке.

## Code review — multi-aspect (security · stability · conventions · docs · regressions · test-coverage · simplification · architecture) **Вердикт: Request changes.** Ревью на реальном коде ветки против `develop` (merge-base `e9409e24`), 6 файлов, +584/−53. Сам фикс корректен для основных сценариев (rename `te`→`ted` на месте, self-heal, детерминированный `findByPageId`, миграция с дедупом и частичным уникальным индексом). Но есть **один блокирующий баг корректности**: подтверждённый перенос адреса (`confirmReassign`) на страницу, у которой уже есть собственный алиас, теперь падает из-за порядка операций относительно нового уникального индекса. Критичных проблем (outage / потеря данных / эксплойт) нет — поэтому Request changes, а не Block merge. Security и Conventions — чисто (LGTM). --- ### ⛔ Must fix before merge **[stability/regressions] Переставьте удаление перед `updatePageId` в ветке подтверждённого reassign** — `share-alias.service.ts:96-108` В ветке «имя занято другой страницей + `confirmReassign`» код сначала делает `updatePageId(byName.id, pageId)` и только потом `deleteOthersForPage`. Новая миграция `20260627T120000` добавляет **обычный (non-deferrable) частичный уникальный индекс** `(workspace_id, page_id) WHERE page_id IS NOT NULL`, который Postgres проверяет немедленно по завершении каждого statement (уникальный *индекс* нельзя сделать `DEFERRABLE`). Реалистичный сценарий: у страницы A адрес `shared`, у страницы B уже есть свой адрес `bee`; пользователь подтверждает перенос `shared` на B. На шаге `updatePageId` появляются две строки с `page_id = B` → немедленный `23505` **до** того, как отработает `deleteOthersForPage`. Транзакция откатывается, а catch на строках 152-153 переводит это в вводящую в заблуждение ошибку `ConflictException('Alias already taken')` — хотя имя как раз было свободно занять. До этого PR ограничения `(workspace_id, page_id)` не было и такой swap работал → это **регрессия ранее работавшего сценария**. **Fix:** в этой ветке сначала вызвать `deleteOthersForPage(pageId, byName.id, workspaceId, trx)` (удалит существующий алиас целевой страницы; `byName.id` пока указывает на старую страницу, поэтому исключение по `keepId` безвредно), затем `updatePageId(byName.id, pageId, workspaceId, trx)` — после этого `byName.id` остаётся единственной строкой для `pageId`, завершающий `deleteOthersForPage` уже не нужен. Заодно это делает истинным JSDoc на строках 50-53 («…the page never transiently has zero or **duplicate** rows»), который сейчас противоречит фактическому порядку в этой ветке (та же неточность продублирована в теле коммита). Добавьте интеграционный тест на этот случай (см. «Test coverage»). - **[warning][stability] Различайте, какой именно индекс дал `23505`** — `share-alias.service.ts:144-157` Catch трактует любой `23505` как «имя уже занято». После добавления второго уникального индекса `(workspace_id, page_id)` это уже не всегда так (гонка двух `setAlias` по одной странице, да и баг выше). **Fix:** анализировать `err.constraint`/`err.detail` — маппить `share_aliases_workspace_id_alias_unique` → «Alias already taken», а `share_aliases_workspace_id_page_id_unique` → отдельный код/сообщение; как минимум логировать `err.constraint`. - **[suggestion][regressions] Опишите в release notes, что дедуп-миграция гасит старые `/l/<old>` ссылки** — `20260627T120000-share-aliases-one-per-page.ts:19-26` `DELETE` в `up()` необратимо удаляет все строки страницы кроме новейшей. Старые слаги резолвятся публичным редиректом через `findByAliasAndWorkspace` (не `findByPageId`), так что ранее живые `/l/<старое-имя>` после апгрейда начнут отдавать SPA-404. Это и есть суть фикса (гасятся ровно дубликаты от старого бага), но удаление тихое и необратимое (`down()` восстанавливает индекс, но не строки). Изменений в коде не требуется — только пометка в описании/чейнджлоге. --- ### Test coverage Покрытие в целом хорошее (56 unit + 6 integration), но есть пробелы: - **(связан с must-fix) Нет теста на подтверждённый reassign, когда у целевой страницы УЖЕ есть свой алиас** — `share-alias-one-per-page.int-spec.ts:181-214`. Текущий «cross-page collision» переносит имя на страницу B *без* собственного алиаса, поэтому баг выше не ловится. Добавьте: A←`shared`, B←`bee`, затем `setAlias({pageId: B, alias: 'shared', confirmReassign: true})` → у B ровно одна строка `shared`, у A — ноль. - **[suggestion] Дедуп-`DELETE` проверяется только на одной странице/одном workspace** — `share-alias-one-per-page.int-spec.ts:94-131`. Скоупинг self-join по `(workspace_id, page_id)` (что DELETE не залезает в чужие страницы/воркспейсы) не проверен; к тому же SQL дублируется инлайном вместо вызова `up()` миграции. Засейте дубликаты для ≥2 страниц (и второго workspace). - **[suggestion] Нет теста на rollback/общую ошибку транзакции** — ветка fallthrough `BadRequestException('Failed to set alias')` (`share-alias.service.ts:155-156`) и откат при ошибке в середине транзакции не покрыты. --- ### Architecture & design (forward-looking, non-blocking) **Инвариант «один алиас на страницу» проверяется БД-индексом в середине транзакции → сервис вынужден аккуратно упорядочивать statements.** Это и есть корень must-fix-бага. оставить частичный индекс, починить порядок в reassign-ветке* (effort: s). Минимально, сохраняет dangling-поведение. **Это и есть рекомендованный must-fix.** **Self-heal `deleteOthersForPage` вызывается в каждой из трёх ветвей записи — признак отсутствующего примитива «ровно один алиас на страницу».** свести `deleteOthersForPage` к одному вызову в конце `setAlias`* (effort: s): безопасно только вместе с фиксом порядка в reassign-ветке. ---

Ghost referenced this issue from a commit

2026-06-27 02:50:10 +03:00

fix(share): order swap delete-before-update and distinguish unique violations

Ghost added 1 commit 2026-06-27 02:50:10 +03:00

fix(share): order swap delete-before-update and distinguish unique violations cf6b78bca1

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>

Ghost force-pushed fix/share-alias-rename from cf6b78bca1 to e682bbccd1

2026-06-27 02:54:29 +03:00

Compare

Ghost referenced this issue from a commit

2026-06-27 02:54:29 +03:00

fix(share): order swap delete-before-update and distinguish unique violations

vvzvlad commented

2026-06-27 03:14:36 +03:00

Code review — PR #227 «fix(share): редактирование кастомного адреса переименовывает строку, а не дублирует» (#226)

Вердикт: Approve with comments (одобрить с замечаниями).

Изменение корректное и хорошо протестированное (56 unit + 6 integration на живом Postgres). Блокирующих и критичных проблем нет — все 8 аспектов (security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture) либо вернули LGTM, либо только неблокирующие suggestion'ы. Ядро фикса — порядок DELETE→UPDATE при свопе относительно non-deferrable частичного индекса (workspace_id, page_id), детерминированный findByPageId, самолечение deleteOthersForPage и миграция-дедуп — проверены трассировкой ветвей и подтверждены.

Объём ревью: диф относительно базы develop (b6630deb..e682bbcc), 7 содержательных файлов.

Must fix before merge

[suggestion][stability] Защитить swap/rename от undefined-строки при гонке с удалением внутри транзакции — apps/server/src/core/share/share-alias.service.ts:126-131 (своп) и :145 (rename).
ShareAliasRepo.updatePageId/updateAlias используют .returning(...).executeTakeFirst(), но типизированы как Promise<ShareAlias>. Если параллельный removeAlias удалит строку byName.id между чтением и UPDATE (READ COMMITTED, разные стейтменты), UPDATE не заматчит строк и вернёт undefined. В swap-ветке это undefined уходит прямо в тело ответа 200 — «успех» без алиаса, нарушение контракта; в rename-ветке undefined разыменовывается (row.id) и падает в общий 400 «Failed to set alias». Окно узкое (нужно удаление именно этой строки в момент свопа), поэтому не блокер.
Fix: трактовать пустой результат UPDATE как гонку — при undefined бросать ConflictException (код вроде ALIAS_PAGE_RACE / «retry»), либо перевести эти методы репозитория на executeTakeFirstOrThrow().
[suggestion][simplification] Убрать избыточный вторичный logger.warn (и мёртвую константу UNIQUE_ALIAS_INDEX) — это же закрывает пробел в покрытии — apps/server/src/core/share/share-alias.service.ts:192-199.
Нарушенный constraint уже безусловно логируется строкой выше (:179-181). Вторичный warn для «неожиданного» индекса не меняет поток управления — обе ветки бросают один и тот же 'Alias already taken', а UNIQUE_ALIAS_INDEX используется только здесь. Это объединяет находку test-coverage: ветка 194-198 — единственная новая в setAlias, не покрытая тестом.
Fix (одно из двух): (а) удалить блок 194-198 и константу UNIQUE_ALIAS_INDEX — тогда непокрытая ветка исчезает; или (б) если оставляете её как ops-сигнал о неизвестном индексе — добавить unit-тест с { code: '23505', constraint_name: 'some_unexpected_index' }, проверяющий 'Alias already taken'.

Test coverage

Покрытие новой логики обширное и осмысленное (ассерты, а не «тривиально зелёные»): rename-in-place (te→ted, дубля нет, старый линк мёртв), same-name no-op, insert, подтверждённый своп с порядком DELETE→UPDATE и удалением прежнего алиаса целевой страницы, ALIAS_REASSIGN_REQUIRED (+currentPageTitle), ALIAS_PAGE_RACE, «Alias already taken», findByPageId newest-wins на legacy-дублях, deleteOthersForPage, миграция-дедуп + частичный уникальный индекс (отвергает второй (workspace_id, page_id), допускает несколько NULL). Эти тесты падали бы на до-фиксовом коде. Единственный незакрытый путь — логирующая ветка 194-198 (см. выше; мотивируется находкой simplification).

Architecture & design (forward-looking, non-blocking)

2. Маппинг Postgres 23505 → ошибка приложения теперь продублирован в двух сервисах (share-alias.service.ts и ai-chat/roles/ai-agent-roles.service.ts; плюс «голая» проверка в favorite.repo.ts).
Дублируется именно хрупкая, привязанная к драйверу часть: SQLSTATE '23505' и чтение имени индекса как err.constraint_name ?? err.constraint (квирк kysely-postgres-js/postgres@3.x). Риск дрейфа: при апгрейде драйвера правку придётся вносить в N мест, ошибка — тихий мисмаппинг.
вынести два хелпера_ isUniqueViolation(err) / violatedConstraint(err) в apps/server/src/database/utils.ts (effort: small). Плюсы: централизует драйвер-специфичную часть в одном месте (там же уже живут jsonbBind/parseJsonbValue по тому же принципу); семантика «какое сообщение» остаётся в каждом сервисе. Минусы: затрагивает второй сервис, расширяя радиус PR.

## Code review — PR #227 «fix(share): редактирование кастомного адреса переименовывает строку, а не дублирует» (#226) **Вердикт: Approve with comments (одобрить с замечаниями).** Изменение корректное и хорошо протестированное (56 unit + 6 integration на живом Postgres). Блокирующих и критичных проблем нет — все 8 аспектов (security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture) либо вернули LGTM, либо только неблокирующие suggestion'ы. Ядро фикса — порядок DELETE→UPDATE при свопе относительно non-deferrable частичного индекса `(workspace_id, page_id)`, детерминированный `findByPageId`, самолечение `deleteOthersForPage` и миграция-дедуп — проверены трассировкой ветвей и подтверждены. Объём ревью: диф относительно базы `develop` (`b6630deb..e682bbcc`), 7 содержательных файлов. ### Must fix before merge - **[suggestion][stability] Защитить swap/rename от `undefined`-строки при гонке с удалением внутри транзакции** — `apps/server/src/core/share/share-alias.service.ts:126-131` (своп) и `:145` (rename). `ShareAliasRepo.updatePageId`/`updateAlias` используют `.returning(...).executeTakeFirst()`, но типизированы как `Promise<ShareAlias>`. Если параллельный `removeAlias` удалит строку `byName.id` между чтением и UPDATE (READ COMMITTED, разные стейтменты), UPDATE не заматчит строк и вернёт `undefined`. В swap-ветке это `undefined` уходит прямо в тело ответа 200 — «успех» без алиаса, нарушение контракта; в rename-ветке `undefined` разыменовывается (`row.id`) и падает в общий 400 «Failed to set alias». Окно узкое (нужно удаление именно этой строки в момент свопа), поэтому не блокер. _Fix:_ трактовать пустой результат UPDATE как гонку — при `undefined` бросать `ConflictException` (код вроде `ALIAS_PAGE_RACE` / «retry»), либо перевести эти методы репозитория на `executeTakeFirstOrThrow()`. - **[suggestion][simplification] Убрать избыточный вторичный `logger.warn` (и мёртвую константу `UNIQUE_ALIAS_INDEX`) — это же закрывает пробел в покрытии** — `apps/server/src/core/share/share-alias.service.ts:192-199`. Нарушенный constraint уже безусловно логируется строкой выше (`:179-181`). Вторичный `warn` для «неожиданного» индекса не меняет поток управления — обе ветки бросают один и тот же `'Alias already taken'`, а `UNIQUE_ALIAS_INDEX` используется только здесь. Это объединяет находку test-coverage: ветка `194-198` — единственная новая в `setAlias`, не покрытая тестом. _Fix (одно из двух):_ (а) удалить блок `194-198` и константу `UNIQUE_ALIAS_INDEX` — тогда непокрытая ветка исчезает; **или** (б) если оставляете её как ops-сигнал о неизвестном индексе — добавить unit-тест с `{ code: '23505', constraint_name: 'some_unexpected_index' }`, проверяющий `'Alias already taken'`. ### Test coverage Покрытие новой логики обширное и осмысленное (ассерты, а не «тривиально зелёные»): rename-in-place (te→ted, дубля нет, старый линк мёртв), same-name no-op, insert, подтверждённый своп с порядком DELETE→UPDATE и удалением прежнего алиаса целевой страницы, `ALIAS_REASSIGN_REQUIRED` (+`currentPageTitle`), `ALIAS_PAGE_RACE`, «Alias already taken», `findByPageId` newest-wins на legacy-дублях, `deleteOthersForPage`, миграция-дедуп + частичный уникальный индекс (отвергает второй `(workspace_id, page_id)`, допускает несколько `NULL`). Эти тесты падали бы на до-фиксовом коде. Единственный незакрытый путь — логирующая ветка `194-198` (см. выше; мотивируется находкой simplification). ### Architecture & design (forward-looking, non-blocking) **2. Маппинг Postgres 23505 → ошибка приложения теперь продублирован в двух сервисах** (`share-alias.service.ts` и `ai-chat/roles/ai-agent-roles.service.ts`; плюс «голая» проверка в `favorite.repo.ts`). Дублируется именно хрупкая, привязанная к драйверу часть: SQLSTATE `'23505'` и чтение имени индекса как `err.constraint_name ?? err.constraint` (квирк `kysely-postgres-js`/`postgres@3.x`). Риск дрейфа: при апгрейде драйвера правку придётся вносить в N мест, ошибка — тихий мисмаппинг. вынести два хелпера_ `isUniqueViolation(err)` / `violatedConstraint(err)` в `apps/server/src/database/utils.ts` (effort: small). Плюсы: централизует драйвер-специфичную часть в одном месте (там же уже живут `jsonbBind`/`parseJsonbValue` по тому же принципу); семантика «какое сообщение» остаётся в каждом сервисе. Минусы: затрагивает второй сервис, расширяя радиус PR.

Ghost added 1 commit 2026-06-27 03:24:26 +03:00

fix(share): show reassign hint instead of dead-end error for a taken custom address 309719abc6

The share modal flagged a custom address already owned by another page with a
red "This address is already in use" error driven by the availability probe.
That reads as terminal even though Save actually triggers the server's
409 `ALIAS_REASSIGN_REQUIRED` and opens the "Move custom address?" confirm
modal that retargets the address to the current page — so the reassign path was
hidden behind what looked like a hard stop.

Replace the red error with an informational description hint ("This address is
in use. Saving will move it to this page.") and keep Save enabled, so the
existing confirm-reassign flow is discoverable. Renaming to a FREE name was
already correct (the probe returns available -> no error -> server renames the
single row in place); this only changes the taken-name presentation.

Verified end-to-end in a real browser against a live stand on this branch:
- A (free rename `test`->`test2`): 200, same alias row renamed in place, link
  becomes `/l/test2`, no error, exactly one DB row for the page.
- B (`test2` owned by another page): hint shown (no dead-end error), Save ->
  409 ALIAS_REASSIGN_REQUIRED -> "Move custom address?" modal -> confirm -> 200,
  the single row retargets, one row each.
- C (same-name re-save): Save disabled (no-op); first-time set inserts.

Add a client component test covering both branches (taken name -> hint not
error + Save enabled; 409 -> reassign modal -> confirm sends confirmReassign).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Ghost referenced this issue from a commit

2026-06-27 03:33:54 +03:00

fix(share): guard alias swap/rename against concurrent-delete race; share unique-violation helpers

Ghost added 1 commit 2026-06-27 03:33:54 +03:00

fix(share): guard alias swap/rename against concurrent-delete race; share unique-violation helpers 767ac9e7e2

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>

vvzvlad merged commit 39113c9dbf into develop

2026-06-27 03:53:33 +03:00

vvzvlad referenced this issue from a commit

2026-06-27 03:53:34 +03:00

Merge pull request 'fix(share): custom address edit renames in place instead of duplicating (#226)' (#227) from fix/share-alias-rename into develop

~~Ghost referenced this pull request 2026-06-28 04:23:39 +03:00~~

WIP: feat(git-sync): native two-way Docmost↔git Markdown sync (Phases A–D, live-verified) #119

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#227