fix(activity-log): stop writing conversation ids into activity_logs.pageId

[2witstudios]

Two stacked defects were silently dropping the audit trail for every Global Assistant message edit and delete.

[ActivityLogger] Failed to log activity: Error: Failed query: insert into "activity_logs" ...
  [cause]: error: ... violates foreign key constraint "activity_logs_pageId_pages_id_fk"
    detail: 'Key (pageId)=(c2npdiezknc9adz0f91masv6) is not present in table "pages".'

The failing INSERT runs inside the transaction holding pg_advisory_xact_lock(ACTIVITY_CHAIN_LOCK_KEY) — the global serialization point for all activity logging — and burns a chainSeq on every failure.

Defect 1 — conversation id written into activity_logs.pageId

apps/web/src/app/api/ai/global/[id]/messages/[messageId]/route.ts passed pageId: conversationId in both PATCH and DELETE. conversationId is validated against the conversations table, which has no relationship to pages, so the FK violation was guaranteed — not a race.

Both call sites now pass pageId: null with a corrected comment. logMessageActivity accepts pageId: string | null, mirroring the driveId: string | null field beside it, and forwards message.pageId ?? undefined.

A repo-wide grep for pageId: conversationId returned exactly these two lines; every other message call site already passes a real page id and is untouched.

pageId: null + driveId: null is the platform's existing shape for user-level activity

This is not a fallback — the consumers already model it as a first-class case, which I traced end to end:

• Authorization (api/activities/[activityId]/route.ts:64-86) branches pageId → page-view check, driveId → drive-member check, else activity.userId !== userId → 403. Global-assistant rows land in that third branch, so the owner can view and roll back their own activity and everyone else gets 403 — fail-closed, no new exposure. api/integrations/providers/install/route.ts:57 already writes user-level rows in exactly this shape.
• Table routing (pickConversationTable, page-mutation-plan.ts:198-207) computes isGlobal from conversationType === 'global' before consulting hasPageId, so rollback/redo/preview keep resolving to the messages table. All three consumers pass hasPageId: !!activity.pageId.
• Rollback planning (planMessageRollback) never reads pageId; the !activity.pageId guards in rollback-plans.ts are on page-oriented plans only.
• Realtime (broadcastActivityEvent) derives its channels from truthy driveId/pageId, so a user-level row broadcasts to nothing — a graceful no-op, not an error, and already the case for existing user-level rows. The message edit/delete broadcasts on this route (broadcastAiMessageEdited/Deleted) are separate and unaffected, so the chat UI still updates live; only the activity-feed event is absent, which is inherent to user-level activity and out of scope here.

Conversation linkage is carried by aiConversationId + metadata.conversationType: 'global', both already passed today.

Defect 2 — the FK-retry fallback never fired in production

logActivity read code / constraint / detail off the top level of the caught error. Drizzle 0.45.2 (drizzle-orm/errors.ts) throws:

export class DrizzleQueryError extends Error {
  constructor(public query: string, public params: any[], public override cause?: Error) {
    super(`Failed query: ${query}\nparams: ${params}`);
    if (cause) (this as any).cause = cause;   // the pg error lives here
  }
}

The wrapper carries no code, constraint, or detail of its own — they are only on .cause. So the guard read undefined for all three and skipped the retry. The production log confirms it: it printed the no-retry branch, not ... after FK retry:.

Notably #1319 ("harden FK error detection") already half-diagnosed this — it observed that Drizzle wraps the error, but concluded it "wraps without forwarding .constraint" and added a detail fallback at the top level. Given the constructor above, detail was undefined there too, so that fallback never fired either. This PR supersedes that fix while keeping the detail signal, now evaluated at whatever depth the pg error actually sits.

Consequence: the page-deleted-mid-log race the fallback was written for (ae2cc0a0a) was unhandled for every caller, not just this route.

New pure module packages/lib/src/monitoring/activity-log-errors.ts exports isPageIdForeignKeyError, which walks a bounded (5-link) .cause chain — the cap also terminates cyclic chains without tracking visited nodes. logActivity now calls it; the && input.pageId condition is unchanged.

Tests (RED first)

Every test below was written and observed failing before the implementation:

• activity-log-errors.test.ts — 16 cases at 100% branch coverage: the exact production nested shape, flat legacy shape, two-level nesting, detail fallback, wrong constraint, wrong code, non-string detail, null/undefined/string/plain-object/bare-Error, a cyclic cause chain, and past the depth limit.
• activity-logger.test.ts — added a nested-cause retry case (expected 2, received 1 against the old guard — the retry was provably skipped) and a pageId: null forwarding case. The five existing flat-error cases are intact and still pass, since flat errors remain supported.
• The two assertions in the global route tests that pinned pageId: mockConversationId failed after the route fix and were corrected to pageId: null.
• page-mutation-plan.test.ts — added the conversationType: 'global' + no pageId case. The suite covered global-with-pageId and no-type-without-pageId, but not the exact shape this PR now emits.

And an integration test, because hand-built errors are what let this ship

Every unit test above constructs the Drizzle wrapper by hand, so it can only show the predicate matches
the shape we believe the driver throws — exactly the gap that hid this bug for two attempts.
activity-logger-fk-retry.integration.test.ts provokes a real 23503 against the real activity_logs
table and lets the real driver wrap it: a real page id round-trips; a well-formed but absent page id
(a page deleted mid-log) still records the activity with only the pageId dropped; and a
global-assistant delete records with pageId null while preserving aiConversationId and
metadata.conversationType.

Confirmed to be a genuine regression test: temporarily restoring the old top-level guard fails the
middle case with expected undefined to be defined — the audit row is dropped entirely, which is the
production bug reproduced without synthesizing anything. ci.yml provisions postgres:17-alpine and
runs db:migrate before turbo run test:coverage, so these execute in CI rather than skipping.

Notes

• No migration. Because Defect 2 suppressed the retry, these rows were never written — there are no orphaned or mislinked activity rows to repair. Schema, FK, advisory lock and hash-chain logic are untouched.
• Out of scope, flagged: logActivityWithTx has no FK retry at all.

Validation

• bun run typecheck — 16/16 packages pass
• bun run lint — 14/14 pass
• bun run test:unit — lib 9230 passed / 0 failed; web 15704 passed / 0 failed (run with TZ=UTC; src/lib/messages/__tests__/grouping.test.ts holds a pre-existing timezone-dependent case unrelated to this change)
• bun run --filter '@pagespace/lib' test:coverage — passes the package's gate (branches ≥94) at
  96.06% statements / 94.84% branches / 88.82% functions. This is what CI's Unit Tests job enforces;
  activity-log-errors.ts itself is at 100% on all four metrics, so it lifts rather than dilutes the
  branch figure, whose margin over the threshold is thin.
• bun x knip — no new findings
• Branch is 0 commits behind master with zero conflicts; see the checks tab for current CI state

A full review of this diff (OWASP pass, consumer-chain verification, and three deferred
standards nits) is recorded at tasks/reviews/pu-activity-log-fk.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AiqgtMFz5y6gJTLn3aLJRd

Summary by CodeRabbit

• Bug Fixes

  • Fixed activity logging for Global Assistant message edits and deletions so entries no longer reference an invalid page.
  • Improved recovery when activity logs encounter missing or deleted page references, preserving other activity details.

• Tests

  • Added coverage for wrapped database errors, global conversations, missing pages, and retry behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ca330fd5-c99a-4d88-8b25-e76cd2c7826e

📥 Commits

Reviewing files that changed from the base of the PR and between 8d7a9e2 and bf19da8.

📒 Files selected for processing (9)

• apps/web/src/app/api/ai/global/[id]/messages/[messageId]/__tests__/route.test.ts
• apps/web/src/app/api/ai/global/[id]/messages/[messageId]/route.ts
• apps/web/src/services/api/rollback/__tests__/page-mutation-plan.test.ts
• packages/lib/src/monitoring/__tests__/activity-log-errors.test.ts
• packages/lib/src/monitoring/__tests__/activity-logger-fk-retry.integration.test.ts
• packages/lib/src/monitoring/__tests__/activity-logger.test.ts
• packages/lib/src/monitoring/activity-log-errors.ts
• packages/lib/src/monitoring/activity-logger.ts
• tasks/reviews/pu-activity-log-fk.md

---

📝 Walkthrough

Walkthrough

Global AI message activity logging now uses null for non-page-backed conversations. The activity logger normalizes this value and detects nested PostgreSQL page foreign-key errors for retry handling, with unit, integration, route, and rollback coverage.

Changes

Activity log foreign-key handling

Layer / File(s)	Summary
Global conversation activity linkage apps/web/src/app/api/ai/global/..., packages/lib/src/monitoring/activity-logger.ts, apps/web/src/services/api/rollback/...	Global PATCH and DELETE activity payloads use pageId: null; the logger omits that field while preserving conversation metadata, with related route and table-selection tests.
Page foreign-key classification and retry packages/lib/src/monitoring/activity-log-errors.ts, packages/lib/src/monitoring/__tests__/*, tasks/reviews/pu-activity-log-fk.md	Nested PostgreSQL page foreign-key errors are classified with bounded traversal, activity inserts retry without pageId, and unit/integration coverage plus review verification are added.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• 2witstudios/PageSpace#211: Introduces the related activity-log pageId retry and null-to-undefined handling pattern.
• 2witstudios/PageSpace#1319: Also improves detection of activity_logs.pageId foreign-key violations.
• 2witstudios/PageSpace#2076: Overlaps on the global AI message PATCH and DELETE handlers.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: activity logging now stops writing conversation IDs into activity_logs.pageId.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pu/activity-log-fk

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[2witstudios]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[2witstudios]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[2witstudios]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.