feat: add message audit foundation #3

2026-07-21T10:45:07+08:00

zheromiku commented

2026-07-21 10:45:07 +08:00

Summary

Add the B1.1 unified identity and message-audit foundation.
Add channel_actors, channel_conversations, and messages in migration 000003_message_audit.
Link turns to request/answer messages and persist the resolved agent ID.
Write inbound user messages and successful generated assistant messages transactionally.
Keep failed turns from creating a false assistant-message record.
Pass message type from DingTalk and HTTP ingress, with safe normalization for text/image/rich text.

Data integrity and idempotency

Upsert channel actors and conversations by tenant/channel/external ID.
Return the actual existing IDs after an upsert conflict.
Prevent duplicate inbound messages using a partial unique external-message index.
Keep actor/conversation/message/turn writes in one BeginTurn transaction.
Keep assistant-message/turn completion writes in one FinishTurn transaction.
Add foreign keys for conversations, actors, replies, and turn request/answer message links.
Provide ordered down migration without cascade deletion.

Security boundaries

Do not persist image DataURI/Base64 payloads.
Do not persist DingTalk download codes, session webhooks, temporary URLs, tokens, prompts, or hidden reasoning.
Keep only the existing safe media metadata audit structure.
WeKnora source and database remain unchanged.

Verification

225 tests pass across 6 targeted packages; 0 failures.
go vet passes for all changed runtime packages.
go build ./cmd/dobotai passes.
git diff --check passes.
Existing migrations 000001 and 000002 are unchanged.
Production DobotAI and WeKnora health checks remained green during development.

Not performed

No migration has been executed against the production DobotAI PostgreSQL database.
No Docker or sudo operation was performed for this feature.
No production service was restarted or deployed.
A real PostgreSQL up/re-run/down rehearsal remains required in a disposable database before merge/deploy approval.

Merge scope

Creating this PR is for review only. Do not merge or deploy until the PostgreSQL rehearsal and final approval are complete.

## Summary - Add the B1.1 unified identity and message-audit foundation. - Add `channel_actors`, `channel_conversations`, and `messages` in migration `000003_message_audit`. - Link `turns` to request/answer messages and persist the resolved agent ID. - Write inbound user messages and successful generated assistant messages transactionally. - Keep failed turns from creating a false assistant-message record. - Pass message type from DingTalk and HTTP ingress, with safe normalization for text/image/rich text. ## Data integrity and idempotency - Upsert channel actors and conversations by tenant/channel/external ID. - Return the actual existing IDs after an upsert conflict. - Prevent duplicate inbound messages using a partial unique external-message index. - Keep actor/conversation/message/turn writes in one `BeginTurn` transaction. - Keep assistant-message/turn completion writes in one `FinishTurn` transaction. - Add foreign keys for conversations, actors, replies, and turn request/answer message links. - Provide ordered down migration without cascade deletion. ## Security boundaries - Do not persist image DataURI/Base64 payloads. - Do not persist DingTalk download codes, session webhooks, temporary URLs, tokens, prompts, or hidden reasoning. - Keep only the existing safe media metadata audit structure. - WeKnora source and database remain unchanged. ## Verification - 225 tests pass across 6 targeted packages; 0 failures. - `go vet` passes for all changed runtime packages. - `go build ./cmd/dobotai` passes. - `git diff --check` passes. - Existing migrations `000001` and `000002` are unchanged. - Production DobotAI and WeKnora health checks remained green during development. ## Not performed - No migration has been executed against the production DobotAI PostgreSQL database. - No Docker or sudo operation was performed for this feature. - No production service was restarted or deployed. - A real PostgreSQL up/re-run/down rehearsal remains required in a disposable database before merge/deploy approval. ## Merge scope Creating this PR is for review only. Do not merge or deploy until the PostgreSQL rehearsal and final approval are complete.

zheromiku added 1 commit 2026-07-21 10:45:08 +08:00

feat: add message audit foundation d857853ea3

zheromiku reviewed 2026-07-21 10:48:37 +08:00

zheromiku left a comment

Hermes review summary

Verdict: changes requested before merge

Critical

None found.

Warnings

internal/qa/service.go:228-233 — stable identity values are only checked with TrimSpace, but the untrimmed values continue into session keys and the audit tables.
Authenticated HTTP clients can therefore create separate actors/conversations/idempotency keys for user-1 and user-1. Canonicalize the stable string identifiers once before BuildKey and BeginTurn (trim only; do not alter case-sensitive external IDs).
internal/qa/service.go:216-222 — the QA pipeline accepts card and feedback message types.
Those values belong to the broader message ledger but should not enter the WeKnora question-answer path. Limit the QA pipeline to text, image, and rich_text; future feedback/card flows should write through their dedicated services.

Suggestions

db/migrations/000003_message_audit.up.sql:20-21,39-40 — redundant lookup indexes.
The two UNIQUE (tenant_key, channel, external_*_id) constraints already create equivalent B-tree indexes. Remove the duplicate explicit indexes to avoid extra storage and write amplification.
db/migrations/000003_message_audit.up.sql:77-78 — correlation index is not tenant-prefixed.
Prefer (tenant_key, correlation_id) so normal audit queries preserve tenant isolation in the index access path.

Required verification before merge

Run 000003 up, repeated up, down, and rollback/order checks against a disposable PostgreSQL 16 database.
Exercise concurrent duplicate BeginTurn writes and verify exactly one inbound message and one turn are created.

Looks good

SQL is parameterized.
Begin/finish audit writes have transaction boundaries.
Duplicate external messages use a partial unique index.
Failed turns do not create assistant messages.
Foreign-key and down-migration ordering are sensible.
No DataURI, download code, webhook, token, temporary URL, prompt, or hidden reasoning is persisted.
Remote PR head matches d857853ea34e44edef8b04e5dd364778b486689f.
225 targeted tests, vet, build, and diff checks passed locally.

No merge or deployment approval is given by this review.

## Hermes review summary **Verdict: changes requested before merge** ### Critical - None found. ### Warnings 1. **`internal/qa/service.go:228-233` — stable identity values are only checked with `TrimSpace`, but the untrimmed values continue into session keys and the audit tables.** Authenticated HTTP clients can therefore create separate actors/conversations/idempotency keys for `user-1` and ` user-1 `. Canonicalize the stable string identifiers once before `BuildKey` and `BeginTurn` (trim only; do not alter case-sensitive external IDs). 2. **`internal/qa/service.go:216-222` — the QA pipeline accepts `card` and `feedback` message types.** Those values belong to the broader message ledger but should not enter the WeKnora question-answer path. Limit the QA pipeline to `text`, `image`, and `rich_text`; future feedback/card flows should write through their dedicated services. ### Suggestions 1. **`db/migrations/000003_message_audit.up.sql:20-21,39-40` — redundant lookup indexes.** The two `UNIQUE (tenant_key, channel, external_*_id)` constraints already create equivalent B-tree indexes. Remove the duplicate explicit indexes to avoid extra storage and write amplification. 2. **`db/migrations/000003_message_audit.up.sql:77-78` — correlation index is not tenant-prefixed.** Prefer `(tenant_key, correlation_id)` so normal audit queries preserve tenant isolation in the index access path. ### Required verification before merge - Run `000003` up, repeated up, down, and rollback/order checks against a disposable PostgreSQL 16 database. - Exercise concurrent duplicate `BeginTurn` writes and verify exactly one inbound message and one turn are created. ### Looks good - SQL is parameterized. - Begin/finish audit writes have transaction boundaries. - Duplicate external messages use a partial unique index. - Failed turns do not create assistant messages. - Foreign-key and down-migration ordering are sensible. - No DataURI, download code, webhook, token, temporary URL, prompt, or hidden reasoning is persisted. - Remote PR head matches `d857853ea34e44edef8b04e5dd364778b486689f`. - 225 targeted tests, vet, build, and diff checks passed locally. No merge or deployment approval is given by this review.

zheromiku added 1 commit 2026-07-21 10:53:03 +08:00

fix: address message audit review findings 32b76b84cb

zheromiku reviewed 2026-07-21 10:53:45 +08:00

zheromiku left a comment

Follow-up review

Commit 32b76b84cbda6b8bed5d95d5d2077c951fc9fae3 resolves all four code-review findings:

Stable identity fields are canonicalized with TrimSpace before resolver, session-key, idempotency, and audit writes; question content remains unchanged.
The QA path now rejects card and feedback before database/model side effects.
Redundant actor/conversation lookup indexes were removed.
Correlation lookup is now tenant-scoped with (tenant_key, correlation_id).

Verification after the fixes:

230 targeted tests pass across 6 packages; 0 failures.
go vet, go build ./cmd/dobotai, and git diff --check pass.
No production database migration, merge, or deployment was performed.

Code findings are resolved. The remaining merge blocker is the disposable PostgreSQL 16 up/re-run/concurrency/down rehearsal.

## Follow-up review Commit `32b76b84cbda6b8bed5d95d5d2077c951fc9fae3` resolves all four code-review findings: - Stable identity fields are canonicalized with `TrimSpace` before resolver, session-key, idempotency, and audit writes; question content remains unchanged. - The QA path now rejects `card` and `feedback` before database/model side effects. - Redundant actor/conversation lookup indexes were removed. - Correlation lookup is now tenant-scoped with `(tenant_key, correlation_id)`. Verification after the fixes: - 230 targeted tests pass across 6 packages; 0 failures. - `go vet`, `go build ./cmd/dobotai`, and `git diff --check` pass. - No production database migration, merge, or deployment was performed. **Code findings are resolved. The remaining merge blocker is the disposable PostgreSQL 16 up/re-run/concurrency/down rehearsal.**

zheromiku reviewed 2026-07-21 11:09:35 +08:00

zheromiku left a comment

PostgreSQL 16 migration rehearsal passed

The remaining review blocker has been cleared against a disposable, loopback-only PostgreSQL 16.14 instance using tmpfs storage.

Results:

Baseline migrations 000001 + 000002: PASS
000003 first up: PASS
000003 repeated up: PASS (expected IF NOT EXISTS notices only)
Tables, foreign keys, tenant-scoped correlation index, and redundant-index absence: PASS
16 concurrent duplicate BeginTurn calls: exactly 1 turn/inbound message created
Concurrent assistant finish idempotency: PASS
Failed turn created no assistant message: PASS
Sensitive media persistence check: PASS
000003 first down: PASS
000003 repeated down: PASS
Baseline tables and 000002.media_json survived down: PASS
Temporary container cleanup and port release: PASS
Production DobotAI and WeKnora health checks remained OK; no restart occurred

All code-review and migration-rehearsal blockers are now cleared. The PR remains open and unmerged for the owner's merge decision. No production migration or deployment was performed.

## PostgreSQL 16 migration rehearsal passed The remaining review blocker has been cleared against a disposable, loopback-only PostgreSQL **16.14** instance using tmpfs storage. Results: - Baseline migrations `000001` + `000002`: PASS - `000003` first up: PASS - `000003` repeated up: PASS (expected `IF NOT EXISTS` notices only) - Tables, foreign keys, tenant-scoped correlation index, and redundant-index absence: PASS - 16 concurrent duplicate `BeginTurn` calls: exactly 1 turn/inbound message created - Concurrent assistant finish idempotency: PASS - Failed turn created no assistant message: PASS - Sensitive media persistence check: PASS - `000003` first down: PASS - `000003` repeated down: PASS - Baseline tables and `000002.media_json` survived down: PASS - Temporary container cleanup and port release: PASS - Production DobotAI and WeKnora health checks remained OK; no restart occurred All code-review and migration-rehearsal blockers are now cleared. The PR remains open and unmerged for the owner's merge decision. No production migration or deployment was performed.

zheromiku merged commit c49cde1042 into main

2026-07-21 11:12:38 +08:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Weknora/dobotai-platform#3