Record every tool call in an audit log - #1554
Open
midego1 wants to merge 10 commits into
Open
Conversation
Executor kept no record of tool usage. A run that called GitHub or Search Console left one HTTP line (POST /mcp 200) and nothing about which integration, which tool, or what came back — and the analytics catalog is anonymous by construction, so it deliberately drops exactly those fields. Two questions were therefore unanswerable after the fact: what did this agent touch, and what did my policies actually stop. `execute` is the one place every call passes through, whatever the plugin kind and whatever the host, so the row is written there, from an `onExit` wrapper that sees every way a call can end: - ok / fail — reached the upstream. `fail` is a tool's own error result, which rides the SUCCESS channel by design and would otherwise be recorded as a healthy call. - blocked / declined — never left the gateway. A policy stopped it, or a human refused the approval. These leave no other trace anywhere: they end before any request is made. - error — the tool or connection did not exist, the plugin failed to load, the transport broke. Arguments and results are never stored: an argument can be a credential. The row keeps the top-level argument NAMES, which is what an audit needs without the table becoming a place secrets accumulate. Writing a row can never change the outcome of the call it describes — a failed write is logged and swallowed, because an audit trail that can take the gateway down with it is worse than one with a gap in it. Readable three ways: `executor.toolCalls.list()`, `GET /api/tool-calls` (filter by integration, connection, outcome, time), and an Activity page in the console. Read-only by construction — a log a caller can edit is not evidence, so there is no write or delete endpoint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A day of agent traffic made the page one long scroll. Same pattern as Admin · Users: 25 rows per page, one extra row fetched to know whether a next page exists (splitPage), Previous/Next with the mono page label. `offset` runs through all three layers — the executor list, the HTTP query (bounded, like every numeric input on this API), and an Atom.family keyed on the offset so paging back is instant while the front page keeps its 5s TTL. The log is append-only at the top, so a page can shift while browsing; that is fine for eyeballing, and programmatic sweeps use `since`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Outcome filter as FilterTabs (All / Ok / Failed / Blocked / Declined / Error) and a search box over the tool address — the one free-text field a row has that is safe to search, because this codebase wrote it rather than an upstream. Both reset to page 1: a new filter is a new list. `search` runs through the same three layers as the other filters: a DB-level `contains` on the executor list (LIKE wildcards in the query are harmless — the match never leaves the caller's own partition), a bounded string on the HTTP query, and the Atom.family key, which now carries `offset|outcome|search`. The input defers the query, not the keystroke. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A Select between the outcome tabs and the search box, listing the tenant's own integration catalog with the same brand marks the Integrations page draws (integrationPresetIconUrl → IntegrationFavicon), so gsc and github tell apart at a glance. Wired to the `integration` filter the API had from the start; "" means all, carried as a sentinel because Select cannot hold an empty value. Like every other filter change it resets to page 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t-log # Conflicts: # packages/core/sdk/src/executor.ts
Upstream's execute-read-concurrency tests (UsefulSoftwareCo#1867) run execute under adversarial scheduler budgets, and with the audit wrapper in place two of them hung the run loop. The A/B pointed at Effect.timeout around the tool-call-log insert. Investigating why exposed the real defect: the write runs in an onExit finalizer, which is uninterruptible, so the timeout's interrupt could never land. The 2s cap this feature shipped with was decorative in exactly the sick-database case it was written for — it protected nothing, and its timer machinery deadlocked under adversarial budgets. Making the write interruptible inside the finalizer did not help; the timer itself is the problem there. So the write is now awaited, deliberately unbounded, and honest about it: bounding a stalled driver is the driver's job; what the wrapper owes the caller is that a row exists before the call returns and that a failed write never changes the call. TOOL_CALL_LOG_WRITE_TIMEOUT is gone from the public surface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… tree Two findings from the PR's first CI run, both guards doing their job: - apps/cloud's org-deletion cascade test walks the schema for every table carrying a `tenant` column and demands each one is seeded and purged. `tool_call_log` joined the schema without joining the purge — deleting an org would have left its audit rows behind. The trail is org data like any other: gone with the org. - check:routes caught that packages/app's committed routeTree.gen.ts (a package added upstream after the last full regen) did not know /activity — that app would have served a 404 for it. The remaining red check (cloud e2e, mcp-session-cap-eviction) times out opening its fourth MCP session; session opens never pass through the audit wrapper, so that one reads as shard flake rather than this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
Executor keeps no record of tool usage. A run that calls GitHub or Search Console leaves one HTTP line —
— and nothing about which integration, which tool, or what came back. There is no
executions/audittable in the schema, andpackages/core/analyticsis anonymous by construction: its own header forbids tool addresses, connection names and arguments, which are exactly the fields an audit needs. Theexecutor.tool.executespan carries the right data, but only where an OTel exporter is configured, and it can't answer "which connection did this agent use last week".Two questions were therefore unanswerable after the fact: what did this agent touch, and what did my policies actually stop.
What this adds
executeinpackages/core/sdk/src/executor.tsis the one place every call passes through — every plugin kind, every host, MCP and REST alike — so the row is written there, from anEffect.onExitwrapper that sees every way a call can end:okfailblockedblockpolicy stopped itdeclinederrorblockedanddeclinedare the rows that make this worth having: both end before any request is made, so nothing upstream ever saw them and no HTTP-level observation can.Each row carries the address as called, its integration/connection/tool, the governing policy (action + pattern), the duration, and the top-level argument names.
Readable three ways:
executor.toolCalls.list({ integration, connection, outcome, since, search, limit, offset })GET /api/tool-callswith the same filtersThe Activity page
Filtering follows the console's existing patterns: outcome as
FilterTabs, an integration dropdown drawn with the same favicon pipeline as the Integrations page, and a search box over the tool address (DB-levelcontains, deferred input). Every filter change resets to page 1.Pagination is the Admin · Users pattern: 25 rows per page, one extra row fetched to answer "is there a next page" without a count query, Previous/Next with the mono page label.
offsetruns through all three layers, bounded at the HTTP edge like every other numeric input.The blocked view — the row that exists nowhere else, because the call ended before any request was made:
Read-only by construction: a log a caller can edit is not evidence, so there is no write or delete endpoint.
executor.toolCalls.prune({ before })exists for retention, and nothing schedules it — an audit log that silently deletes itself on a default nobody chose seemed worse than one that grows.What it deliberately does not store
Arguments, results, and any text that came from outside:
code; plugins deriveerror.messagefrom upstream response bodies (the OpenAPI plugin lifts it straight out), which routinely echo the request back — token included.ToolError.codeis typed as any string, so a plugin can forward a body into it. Anything not matching an identifier shape is dropped; theoutcomecolumn already says what happened.executetakesunknownargs, so{ "ghp_realtoken": null }is a reachable shape; names are length-bounded, identifier-shaped, and credential-shaped ones are dropped.Failure behaviour
Writing a row can never change the outcome of the call it describes. The write is wrapped in
catchCause, so an insert failure — or a defect — is logged and swallowed. It is awaited on purpose (a forked write would be interrupted when a per-request host tears the executor down, and a silently missing row is the one thing an audit log may not do), and deliberately carries no timeout of its own: anonExitfinalizer runs uninterruptible, so a timeout there can never deliver its interrupt — it is decorative in exactly the sick-database case it would be written for, and its timer deadlocks the run loop under the adversarial scheduler budgets the execute-read-concurrency tests (#1867) exercise. Bounding a stalled driver is the driver's job.Scope and follow-ups
ensureDrizzleRuntimeSchemaFromTablescreates the table at boot fromcoreTables. Cloud gets0016_nosy_expediter.sql.apps/local/src/db/executor-schema.tsis left alone — it already predatesartifact/subject, and it only drives a generate-time baseline.(tenant, created_at)index and the schema layer has no non-unique index API yet. Worth adding before this table gets large; I left a note at the table definition rather than adding a unique index whose leading columns wouldn't serve the query.main(last merged through the concurrent-read work of Run independent tool-call reads concurrently #1867; the audit wrapper is re-anchored in the new fiberedexecutepath and the full sdk suite — including the adversarial scheduler tests — passes with it in place).Verification
packages/core/sdk/src/tool-call-log.test.ts— 19 tests: outcome classification per ending (including a decline raised inside a handler, which arrives wrapped inToolInvocationError), the redaction rules with real-looking secrets, list filtering/search/paging tiling without overlap, and executor-level assertions that a blocked call and a declined approval each leave a row.format:check,lint,typecheck(44/44) clean;packages/core/sdk613+,packages/core/api98,packages/react320 tests pass.codex reviewover two rounds; every finding from both is addressed in the branch.🤖 Generated with Claude Code