# История правок shared-sqs — 2026-10-11 (внешнее код-ревью и фиксы)

> Запись по требованию «документировать всё, включая собственные ошибки» (приказ 2026-09-24).
> Предыдущие: `HISTORY/2026-10-10-session-log.md`, `doc/thinking/*-2026-10-08.md`.

## 1. Задача

Провести **внешнее код-ревью** двух сервисов (SQS-service + IoT) моделью Claude Sonnet
(в режиме Plan — правки не вносит, только находит), затем своими силами исправить
найденное. Промпт для ревью — `CLAUDE-REVIEW-PROMPT.md` в корне монорепо
(со списком файлов, фокус-точками и строгим форматом ответа; явно запрещены
правки, оценка тестов/UI/legacy/стиля).

Ревью дало **40 пунктов** (10 🔴 / 29 🟡 / 1 🟢). Разбор вопросов ревьюера и
уточнение некорректных выводов — в п.3.

## 2. Что исправлено (SQS-service)

### 🔴 Критичные (10/10)

| Пункт | Файл | Суть | Коммит |
|-------|------|------|--------|
| 🔴1 | `app/admin/admin.go` | `jwtMiddleware` пропускал любой путь с суффиксом `/auth` (обход JWT + IDOR на `DELETE .../queues/auth`). Теперь — точное сравнение `/ui/api/auth` | `1241093` |
| 🔴2 | `app/gosqs/queue_attributes.go` | Запрет self-/циклического `RedrivePolicy`: перенос в DLQ берёт второй `mu` под первым → deadlock | `9b9bbe1` |
| 🔴3 | `app/gosqs/gosqs.go`, `models.go`, `change_message_visibility*.go` | Перенос в DLQ не персистился (сообщение «воскрешало» в исходной и пропадало из DLQ) | `1e334c2` |
| 🔴4 | `app/persistence/redis.go`, `app/tenant/tenant_store.go` | `DeleteTenant` не чистил индекс `ssq:tenant_ak` — отозванный тенант возвращался через read-through | `f68b0a4` |
| 🔴5 | `app/persistence/redis.go`, `app/cmd/goaws.go` | Глобальный `Client` перезаписывался в `reconnect()` без синхронизации (data race). → `atomic.Pointer` | `d03ce36` |
| 🔴6 | `app/conf/config.go` | `return ports` без `SyncQueues.Unlock()` при ошибке `setQueueRedrivePolicy` → вечный deadlock | `1520753` |
| 🔴7 | `app/billing/billing.go` | `Close()` писал `db=nil` параллельно с `usageWorker` (nil-паника). → мьютекс + остановка воркера до `db.Close` | `4e36ba0` |
| 🔴10 | `app/auth/auth_middleware.go` | `X-Amz-Content-Sha256` не сверялся с телом — подмена тела под подписанным заголовком | `2918a41` |

(🔴8/🔴9 — на стороне IoT, см. `IoT/HISTORY/2026-10-11-session-log.md`.)

### 🟡 Важные

| Пункт | Файл | Суть | Коммит |
|-------|------|------|--------|
| 🟡20 | `app/admin/admin.go` | `createTenantQueue` не валидировал имя очереди (`:`,`/` ломали ключ) | `efd8c8e` |
| 🟡23 | `app/auth/jwt.go` | Токен без `exp` принимался бессрочно → требуем `exp` | `efd8c8e` |
| 🟡24 | `app/leader/leader.go` | При сбое `RefreshLock` инстанс оставался лидером (split-brain) → сброс лидерства после TTL | `efd8c8e` |
| 🟡21 | `app/tenant/tenant_store.go` | Запрет `:` в BYOC `access_key` (коллизии ключа `{ak}:{name}`) | `dc4815d` |
| 🟡22 | `app/admin/admin.go` | `sendMessageToQueue` без лимитов + async persist → лимит `MaxMessagesForQueue` + `SaveMessageSync` | `dc4815d` |
| 🟡33 | `app/models/models.go` | `InitDuplicatation` падал на nil-map `Duplicates` → ленивая инициализация | `dc4815d` |
| 🟡35 | `app/models/models.go` | `Reset()` не чистил `FIFOMessages` (после purge группы не выдавались) | `dc4815d` |
| 🟡36 | `app/gosqs/receive_message.go` | `ApproximateReceiveCount` был завышен на 1 | `dc4815d` |
| 🟡38 | `app/cmd/goaws.go` | `billing.Close()` вызывался ДО `Shutdown` серверов → перенесён после | `dc4815d` |
| 🟡26 | `app/gosqs/delete_message_batch.go` | Ключ map по `ReceiptHandle` (два entry с одним handle теряли Id) → по `Id` | `e0b5f24` |
| 🟡27 | `app/gosqs/send_message.go` | `DelaySeconds` не валидировался → 0–900 | `e0b5f24` |
| 🟡25 | `app/gosqs/send_message_batch.go` | Dedup регистрировался после цикла (два entry с одним id оба добавлялись); `NextSequenceNumber` до проверки дубликата; per-entry `DelaySeconds` не применялся | `6f69755` |
| 🟡29 | `app/gosqs/delete_message*.go`, `persistence/redis.go` | Ошибка `HDel` только логировалась (клиент получал 200, после рестарта сообщение воскрешало) → `DeleteMessage(s)Persist` возвращает ошибку, 500 клиенту | `5d38118` |
| 🟡31 | `app/persistence/redis.go`, `app/cmd/goaws.go` | Ошибка загрузки сообщений очереди давала `continue` (очередь стартовала пустой) → fail-fast (`log.Fatalf`) | `5d38118` |
| 🟡30 | `app/persistence/redis.go` | Async `SaveMessage` без порядковой нумерации (старое состояние могло перезаписать новое) → seq по `{queueKey}|{uuid}` + очистка при удалении | `7a2340d` |
| 🟡32 | `app/gosqs/gosqs.go` | `PeriodicTasks` без `recover` (паника убивала процесс) | `dc4815d` |
| 🟢 | `app/utils/utils.go` | `SqsErrors[errKey]` без проверки → nil-deref на неизвестном ключе | `5d38118` |
| (🟡 new) | `app/gosqs/create_queue.go` | Очередь из API получала `VisibilityTimeout=0` и пр. (дефолты не подставлялись) | `5d38118` |

### Осознанно НЕ менялось — РЕШЕНИЯ (2026-10-11)

- 🟡34 (атрибуты «0 = не передано»): **решение — не менять** совместимость.
  Различать «не передано» и явный 0 потребовало бы перевести `QueueAttributes`
  на указатели — ломает JSON-протокол и Terraform-drift. Явный 0 — редкий сценарий.
- 🟡37 (дефолт `SHARED_SQS_MAX_HEAP_MB`): **решение — оставить выключенным (0)**.
  Безопасное число зависит от RAM конкретной ВМ и числа тенантов — выбор владельца,
  не хардкод в коде.
- 🟡28 (сеть под `queue.mu`): **решение — оставить как есть (вариант A)**. См. п.4.

## 3. Уточнение выводов ревьюера

Ревьюер задал вопросы по коду, ответы найдены в коде (файл:строка):

- **ACL EMQX — единственный барьер для топиков** `{ns}/telemetry/{deviceId}`? Да:
  `bridge/handler.go` берёт `ns`/`deviceID` из топика без сверки принадлежности,
  барьер — EMQX authz (`emqx/emqx.conf`). Добавлена defense-in-depth проверка
  формата (IoT).
- **Self-/циклический `RedrivePolicy` реально задаётся через API?** Да
  (`setQueueAttributesV1` не проверял) — отсюда фикс 🔴2.
- **Пустой `adminToken`?** Недопустим — `goaws.go:~74` `log.Fatal` при отсутствии.
- **Дефолты очереди расходятся** (`create_queue.go` vs `conf/config.go:100-109`)?
  Да — отсюда фикс (🟡 new).
- **`utils.go` паника на неизвестном ключе?** Текущей паники нет (все ключи есть
  в `models.SqsErrors`), но добавлена защита.

Также ревьюер отметил некорректный пункт (удалён из списка по лимиту 40): nil-map
`Duplicates` (🟡33) фактически не воспроизводился в текущем коде — но защита
добавлена как дешёвая страховка.

## 4. 🟡28 — РЕШЕНИЕ (2026-10-11)

- **Проблема**: синхронный `SaveMessageSync` (до 3 с) выполняется под `queue.mu`
  (`send_message.go:150`, `delete_message.go`, `admin.go`). Медленный Redis блокирует
  всю очередь.
- **Варианты**: A) оставить; B) укоротить таймаут Redis-записи; C) полный рефакторинг
  с per-message синхронизацией.
- **Выбрано A — оставить.** Обоснование:
  1. in-memory ядро очередей — одиночка, блокировка `queue.mu` локальна (одна очередь).
  2. Синхронный персист под `mu` = гарантия «нет потери подтверждённого сообщения
     при крахе пода» — приоритет проекта (надёжность, а не масштабирование).
  3. Вынос сети за `mu` вносит окно между `Unlock`/`Lock`: два конкурентных
     `SendMessage` с одним `deduplicationId` оба пройдут `IsDuplicate` (дубль),
     либо при откате `dedup` теряется сообщение — хуже кратковременной блокировки.
  4. Redis-авария — глобальная: деградирует всё, 2–3 с на очередь не критично.
- 🟡34/🟡37 — см. п.2.

## 5. Верификация

- `go build ./...`, `go vet ./...`, `go test -race ./...` — зелёные на всех коммитах.
- Push: `2c6d2d7..f80e1b4` в `origin/master`.

## 6. Деплой на прод (ВМ 213) и smoke

- `make build` → `scp shared-sqs → /home/naeel/sqs-test/shared-sqs.new` →
  `systemctl stop shared-sqs; mv .new → shared-sqs; start`. Бэкап — `shared-sqs.prev`.
- Проверка: `systemctl is-active` = active; `/health` → 200 `v0.1.35`;
  в логе «загружено 5 очередей из Redis» (fail-fast-загрузка сработала, не упала).
- Smoke по `https://sqs.sinefo.ru` (`tests/api_test.sh`, aws cli, тенант `demo-service`):
  **PASS=22 FAIL=0** (17 базовых операций + FIFO CreateQueue/Send/Receive/Delete/DeleteQueue).
  Вывод: 18 фиксов ревью не сломали AWS-совместимость на проде.

## 7. Образы → Gitea registry

- Реестр — **Gitea** `gitea.services.ngcloud.ru` (НЕ Docker Hub). Makefile/Dockerfile/доки исправлены.
- Залито: `nail/shared-sqs:v0.1.35` (+latest; было только v0.1.25), `nail/iot-service:v0.1.13`,
  `nail/iot-emqx:v0.2.4`.
- Push: `e30bf74..f80e1b4`.