Skip to content

feat(audit): attribute workflow audits to their run and merge a run's repeated row audits - #9404

Merged
waleedlatif1 merged 3 commits into
stagingfrom
feat/audit-executor-attribution-coalesce
Oct 27, 2026
Merged

waleedlatif1 merged 3 commits into
stagingfrom
feat/audit-executor-attribution-coalesce

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Executor-delegated audit actors now carry workflowId and executionId, so a workflow-made audit entry traces back to its workflow and run (before, only a per-call delegationId was recorded)
  • New recordAuditRollup / flushAuditRollups in @sim/audit: repeats of the same event within a 2s window (max 100 per row) become one row. Nothing is dropped:
    • a lone occurrence is written exactly as it is today
    • a merged row sums the counters, collects every identifier (rowIds, delegationIds), and records rollup: { entries, firstAt, lastAt }
  • Table row audits (table.updated from insert/update/delete/upsert/replace) opt in, but only for an executor delegation that names its run and has no client request. They merge only within the same execution, table, op, and metadata; session, API key, CLI, and copilot writes are unchanged
  • Server-side credential.accessed (no client request, e.g. a run refreshing a token per tool call) merges per actor + credential; requests with a client still write one row each
  • No-loss guarantees:
    • executeWorkflowCore awaits flushAuditRollups() before returning, so a background run's process can't freeze or exit with rows pending; the flush also waits for writes already in flight
    • window timers are ref'd, so a natural process exit still writes them
    • each pending row is its own insert, so one bad row can't take others with it
    • created_at stays the DB default, so drains' 5-minute stability cursor can't skip a delayed row
    • the surface is captured when the event is recorded, not when it's flushed
  • Docs: audit-logs page describes merged entries and the new actor fields

Type of Change

  • New feature

Testing

  • row-audit.integration.ts (real Postgres): 3 new cases
    • one run's repeated inserts → 1 row with all row ids, summed count, distinct delegation ids
    • two runs never merge
    • a lone run write keeps today's exact shape
    • all 3 fail with the rollup branch disabled and pass with it; the 8 existing cases still pass
  • Unit: principal.test.ts, token-resolution.test.ts (negative cases now also assert no rollup), authorized-workspace-use-case.test.ts, execution-core.test.ts, rows.test.ts, selectors (427 tests)
  • bun run lint; type-check and check:audits left to CI

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing (new tests pass the test-audit authoring gate)
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Oct 27, 2026 9:46pm UTC

Request Review

@greptile-apps

greptile-apps Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the updated test matches the actor produced by the deployment code.

Summary

Adds workflow and run details to delegated audit actors and merges repeated table-row changes and server-side credential accesses.

  • Workflow audit entries identify the workflow and run behind each action.
  • Repeated table changes from one workflow run merge into counted audit entries.
  • Server-side credential accesses without a request can share a counted audit entry.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Run[Workflow run] --> Scope[Run-owned audit scope]
  Scope --> Events[Table changes and credential accesses]
  Events --> Merge[Merge matching repeats]
  Merge --> Write[Write audit rows]
  End[Run finishes] --> Flush[Write remaining rows]
  Flush --> Wait[Wait for this scope's writes]
  Write --> Wait
  Wait --> Return[Return run result]
Loading

Reviews (3) · Last reviewed commit: "test(workflows): expect run attribution ..." · Reviewed by Greptile

Comment thread packages/audit/src/rollup.ts Outdated
Comment thread packages/auth/src/principal.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 14 files

Confidence score: 3/5

  • writePending in packages/audit/src/rollup.ts starts all row inserts at once, which can create connection and actor-lookup pressure on large flushes. Write rows in batches, as log.ts does.
  • The process-global flush in execution-core.ts can drain another workflow’s pending rollup and split events that should be grouped within the two-second window. Scope the flush to the current executor.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/audit/src/rollup.ts">

<violation number="1" location="packages/audit/src/rollup.ts:167">
P2: `writePending` starts every row's insert at once. A flush of up to 1000 rows can open many pooled connections and actor lookups concurrently, which the batch writer in `log.ts` was written to avoid. Write rows in bounded chunks, for example through `insertAuditLogBatch` or a small concurrency limit.</violation>
</file>

<file name="apps/sim/lib/workflows/executor/execution-core.ts">

<violation number="1" location="apps/sim/lib/workflows/executor/execution-core.ts:621">
P2: This flush is process-global, so one workflow can prematurely drain another workflow's pending rollup and split events that occur within the two-second window into separate rows. Flush only rollups owned by this execution, or make the rollup state/run boundary explicit instead of draining the global map here.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic

Comment thread packages/auth/src/principal.ts
Comment thread packages/audit/src/rollup.ts
Comment thread apps/sim/lib/workflows/executor/execution-core.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 15 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 16 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit a9d5dc8 into staging Oct 27, 2026
69 checks passed
@waleedlatif1
waleedlatif1 deleted the feat/audit-executor-attribution-coalesce branch October 28, 2026 02:31

This branch was previously deployed

1 inactive deployment
Preview — 65b190f4 Deployed Oct 27, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant