Repository navigation
feat: add audit logging to avatar, calendar, workflow, task status & channel routes - #778
2witstudios wants to merge 1 commit into
Conversation
…nd channel routes Add fire-and-forget activity logging across 10 API route files using the getActorInfo().then() pattern to avoid blocking responses. Extend the activity resource enum with 'workflow' and 'calendar_event' types. Eliminate redundant page.driveId DB queries in task status audit logging. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds comprehensive fire-and-forget audit logging to API routes handling avatar updates, calendar events, attendees, channels messages, reactions, workflows, and task statuses. It extends the activity logging system to support 'workflow' and 'calendar_event' resource types via enum additions in the database schema and activity-logger types. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3d2bcf7ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| logMessageActivity(userId, 'create', { | ||
| id: createdMessage.id, | ||
| pageId, | ||
| driveId: null, |
There was a problem hiding this comment.
This new audit entry is recorded with driveId: null, so channel message activity is detached from its actual drive. Drive-scoped activity queries filter on activityLogs.driveId (apps/web/src/app/api/activities/route.ts uses eq(activityLogs.driveId, params.driveId) in drive context), which means these records are omitted from drive audit/history views; the same driveId: null pattern added in the reactions route has the same effect. It also suppresses event-triggered workflows because emitWorkflowEvent returns early when event.driveId is missing (apps/web/src/lib/workflows/event-trigger.ts).
Useful? React with 👍 / 👎.
| operation: 'create', | ||
| resourceType: 'page', | ||
| resourceId: pageId, | ||
| driveId: null, |
There was a problem hiding this comment.
These task-status audit logs are also written with driveId: null, which makes them invisible in drive-level audit/history endpoints that filter strictly by drive ID (apps/web/src/app/api/activities/route.ts drive context). Because workflow event dispatch drops events without a drive (apps/web/src/lib/workflows/event-trigger.ts), these operations also cannot trigger any drive-scoped event workflows, even though they are page changes inside a drive.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/src/app/api/channels/[pageId]/messages/[messageId]/reactions/route.ts (1)
94-106: Consider semantic clarity of resourceType for reactions.Using
resourceType: 'message'withoperation: 'create'for adding a reaction may cause confusion in audit logs—the message itself isn't being created. Themetadata.action: 'reaction_added'disambiguates, but consider whether a dedicatedresourceType: 'reaction'would improve audit query clarity.This is a minor consistency consideration; the current approach works functionally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/channels/`[pageId]/messages/[messageId]/reactions/route.ts around lines 94 - 106, Update the audit log entry for reaction events to use a clearer resource type: when calling getActorInfo(...).then(...) and invoking logActivity({ userId, ...actorInfo, operation: 'create', resourceType: 'message', resourceId: messageId, pageId, metadata: { action: 'reaction_added', emoji } }), change resourceType to 'reaction' (and consider changing resourceId to a reaction identifier if available, otherwise keep messageId but document that resourceId references the parent message) so audit queries clearly reflect that a reaction was created; adjust only the logActivity payload in the reactions route where those symbols are used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/api/channels/`[pageId]/messages/route.ts:
- Around line 171-179: The audit log currently hardcodes driveId: null when
calling logMessageActivity after getActorInfo; change this to pass the real
drive context by ensuring you have the channel/page driveId before
logging—either move the getActorInfo.then(...) block to after the channel lookup
(where channel.driveId is available) or perform a small pages lookup to fetch
the page's driveId and use that value instead of null when constructing the
payload for logMessageActivity (referencing createdMessage.id, pageId, and the
resolved driveId).
In `@apps/web/src/app/api/workflows/`[workflowId]/run/route.ts:
- Around line 78-90: The audit logging currently runs only after successful
executeWorkflow; update the handler so audit entries are written even when
executeWorkflow throws by invoking getActorInfo(...) and logActivity(...) in a
finally-like path: wrap the call to executeWorkflow(...) in try/catch and always
call getActorInfo(auth.userId).then(actorInfo => logActivity({...})) in both
success and error cases (setting metadata.action='manual_run',
metadata.success=true/false, include error message/stack when failed), and
ensure any promise rejections from getActorInfo or logActivity are caught (e.g.,
.catch(() => {})) so auditing is fire-and-forget and does not alter the response
flow; reference executeWorkflow, getActorInfo, logActivity, workflowId, and
workflow.name when making the audit payload.
---
Nitpick comments:
In
`@apps/web/src/app/api/channels/`[pageId]/messages/[messageId]/reactions/route.ts:
- Around line 94-106: Update the audit log entry for reaction events to use a
clearer resource type: when calling getActorInfo(...).then(...) and invoking
logActivity({ userId, ...actorInfo, operation: 'create', resourceType:
'message', resourceId: messageId, pageId, metadata: { action: 'reaction_added',
emoji } }), change resourceType to 'reaction' (and consider changing resourceId
to a reaction identifier if available, otherwise keep messageId but document
that resourceId references the parent message) so audit queries clearly reflect
that a reaction was created; adjust only the logActivity payload in the
reactions route where those symbols are used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc5f8329-087c-40cf-907d-fbe41baea101
📒 Files selected for processing (15)
apps/web/src/app/api/account/avatar/route.tsapps/web/src/app/api/calendar/events/[eventId]/attendees/route.tsapps/web/src/app/api/calendar/events/[eventId]/route.tsapps/web/src/app/api/calendar/events/route.tsapps/web/src/app/api/channels/[pageId]/messages/[messageId]/reactions/route.tsapps/web/src/app/api/channels/[pageId]/messages/route.tsapps/web/src/app/api/pages/[pageId]/tasks/statuses/route.tsapps/web/src/app/api/workflows/[workflowId]/route.tsapps/web/src/app/api/workflows/[workflowId]/run/route.tsapps/web/src/app/api/workflows/route.tspackages/db/drizzle/0091_ambitious_eternity.sqlpackages/db/drizzle/meta/0091_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/monitoring.tspackages/lib/src/monitoring/activity-logger.ts
| // Audit logging (fire-and-forget) | ||
| getActorInfo(userId).then(actorInfo => { | ||
| logMessageActivity(userId, 'create', { | ||
| id: createdMessage.id, | ||
| pageId, | ||
| driveId: null, | ||
| conversationType: 'channel', | ||
| }, actorInfo); | ||
| }).catch(() => {}); |
There was a problem hiding this comment.
Pass the real drive context in message audit logs.
At Line 176, driveId is hardcoded to null even for channel messages. This drops drive-level context for audit queries and indexing. Please pass the actual channel/page driveId instead.
💡 Suggested fix
- // Audit logging (fire-and-forget)
+ // Audit logging (fire-and-forget) - include actual drive context
getActorInfo(userId).then(actorInfo => {
logMessageActivity(userId, 'create', {
id: createdMessage.id,
pageId,
- driveId: null,
+ driveId: channel?.driveId ?? null,
conversationType: 'channel',
}, actorInfo);
}).catch(() => {});If channel is only loaded later, move this block to run after the existing channel lookup (Line 237+) or do a small pages lookup before logging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/api/channels/`[pageId]/messages/route.ts around lines 171 -
179, The audit log currently hardcodes driveId: null when calling
logMessageActivity after getActorInfo; change this to pass the real drive
context by ensuring you have the channel/page driveId before logging—either move
the getActorInfo.then(...) block to after the channel lookup (where
channel.driveId is available) or perform a small pages lookup to fetch the
page's driveId and use that value instead of null when constructing the payload
for logMessageActivity (referencing createdMessage.id, pageId, and the resolved
driveId).
| // Audit logging (fire-and-forget) | ||
| getActorInfo(auth.userId).then(actorInfo => { | ||
| logActivity({ | ||
| userId: auth.userId, | ||
| ...actorInfo, | ||
| operation: 'update', | ||
| resourceType: 'workflow', | ||
| resourceId: workflowId, | ||
| resourceTitle: workflow.name, | ||
| driveId: workflow.driveId, | ||
| metadata: { action: 'manual_run', success: result.success, durationMs: result.durationMs }, | ||
| }).catch(() => {}); | ||
| }).catch(() => {}); |
There was a problem hiding this comment.
Manual-run failures are not audited when execution throws.
If executeWorkflow throws (Line 53-Line 60), the handler returns before this new logging block runs, so failed run attempts are missing from audit history.
💡 Suggested fix
+ const fireRunAuditLog = (success: boolean, durationMs?: number, error?: string) => {
+ getActorInfo(auth.userId).then(actorInfo => {
+ logActivity({
+ userId: auth.userId,
+ ...actorInfo,
+ operation: 'update',
+ resourceType: 'workflow',
+ resourceId: workflowId,
+ resourceTitle: workflow.name,
+ driveId: workflow.driveId,
+ metadata: { action: 'manual_run', success, durationMs, error },
+ }).catch(() => {});
+ }).catch(() => {});
+ };
+
try {
result = await executeWorkflow(workflow);
} catch (error) {
const errorMsg = error instanceof Error ? error.message : String(error);
+ fireRunAuditLog(false, undefined, errorMsg);
await db
.update(workflows)
.set({ lastRunStatus: 'error', lastRunError: errorMsg })
.where(eq(workflows.id, workflowId));
return NextResponse.json({ success: false, error: errorMsg }, { status: 500 });
}
- // Audit logging (fire-and-forget)
- getActorInfo(auth.userId).then(actorInfo => {
- logActivity({
- userId: auth.userId,
- ...actorInfo,
- operation: 'update',
- resourceType: 'workflow',
- resourceId: workflowId,
- resourceTitle: workflow.name,
- driveId: workflow.driveId,
- metadata: { action: 'manual_run', success: result.success, durationMs: result.durationMs },
- }).catch(() => {});
- }).catch(() => {});
+ fireRunAuditLog(result.success, result.durationMs, result.error ?? undefined);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/api/workflows/`[workflowId]/run/route.ts around lines 78 -
90, The audit logging currently runs only after successful executeWorkflow;
update the handler so audit entries are written even when executeWorkflow throws
by invoking getActorInfo(...) and logActivity(...) in a finally-like path: wrap
the call to executeWorkflow(...) in try/catch and always call
getActorInfo(auth.userId).then(actorInfo => logActivity({...})) in both success
and error cases (setting metadata.action='manual_run',
metadata.success=true/false, include error message/stack when failed), and
ensure any promise rejections from getActorInfo or logActivity are caught (e.g.,
.catch(() => {})) so auditing is fire-and-forget and does not alter the response
flow; reference executeWorkflow, getActorInfo, logActivity, workflowId, and
workflow.name when making the audit payload.
1 participant
Summary
getActorInfo().then()pattern) across 10 API route files covering avatar, calendar events, calendar attendees, workflows, task statuses, channel messages, and reactionsworkflowandcalendar_eventtypes (schema + migration)db.query.pages.findFirst()calls in task status audit logging, usingdriveId: nullinstead (pageId already provides sufficient audit context)Test plan
pnpm typecheck— full build passes with 0 errors🤖 Generated with Claude Code
Summary by CodeRabbit