feat(engines): SqliteAuditLogger + boot wiring (V1 follow-up #19)#14
Merged
Conversation
Closes the audit-persistence gap left by V1 (PR #13). Adds a sqlite writer wired into gateway boot so tool calls from ollama/openai land in tool_call_log alongside cost_log and sessions. Narrow scope per the agreed-upon order — this PR does NOT change engine, tool, or agent-loop behavior. It only adds persistence to the existing AuditLogger interface defined in V1. migrations/0007_tool_call_log.up.sql (+ .down.sql): Schema mirroring the V1 AuditRow contract (engines/audit.ts): id, session_id, engine, tool_name, args_summary, duration_ms, exit_code, http_status, error, result_truncated, result_bytes, created_at Plus indexes on (session_id, created_at), (engine, tool_name, created_at), and partial index on (error) WHERE error IS NOT NULL for forensic / cost-attribution queries. engines/sqliteAuditLogger.ts (~60L): - Implements AuditLogger from V1 - Prepares INSERT once at construction; reuses across all record() calls (better-sqlite3 prepared-statement pattern, matches sessions/registry.ts style) - sessionId / engineName fall back to "unknown" if AuditRow doesn't carry them (defensive — the agent loop populates them from toolContext, so 'unknown' should only appear if someone bypasses the loop wiring) - Throws if the table is missing at construction (migration not applied → caller sees fail-fast, not a silent skip) engines/audit.ts: - AuditRow extended with optional sessionId + engineName fields. V1 callers (tests, the loop itself when scope is omitted) still compile — the fields default to undefined. - buildAuditRow() gains an optional scope param. Always populates the new keys (possibly as undefined). engines/agentLoop.ts: - The one call site passes scope from opts.toolContext.sessionId / .engineName. ToolExecutionContext already carried these from V1; they were just being dropped on the floor. gateway/server.ts: - Constructs SqliteAuditLogger once when at least one HTTP engine is configured. Reuses initDb() (which auto-applies migration 0007 via the existing migrate-runner). - Same instance passed to both OllamaEngine and OpenAIEngine via their existing { audit } constructor option. Claude-only deployments don't pay for the import. Tests (15 new): audit.test.ts (+2): scope passthrough populates sessionId+engineName; omission leaves them undefined. Updated whitelist to include the new keys. sqliteAuditLogger.test.ts (+13): - insert shape for read/bash/webfetch row variants - multiple inserts produce distinct rows + unique ids - 'unknown' fallback when scope is missing - SECURITY: schema has no content/stdout/stderr/body/response_body columns (negative assertion) - sentinel string sanity check ("THIS_IS_FILE_BODY_..." not in serialized row) - redacted argsSummary survives the round-trip (sanitization is upstream, not this layer's responsibility) - constructor throws if migration unapplied (no silent skip) - record() throws if DB closed mid-session — confirms the loop's safeAudit() has something to catch and forward to logger.warn - DB integrity preserved after a UNIQUE collision; subsequent record() still works - indexes exist on session, engine+tool, and error columns Full package suite: 722/722 (up from 707 in V1). Co-Authored-By: Claude Opus 4.7 (1M context) <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
1 participant
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.
Summary
Closes the audit-persistence gap from V1 (PR #13) so the gateway writes one
tool_call_logrow per tool invocation by the ollama/openai engines. No engine, tool, or agent-loop behavior change — only persistence wired through the existingAuditLoggerinterface.This is the prerequisite for follow-ups hristo2612#20 (manusiawi shadow migration) and hristo2612#21 (report-url triage migration).
What changes
migrations/0007_tool_call_log.up.sql(+.down.sql) — schema mirroring V1'sAuditRow: id, session_id, engine, tool_name, args_summary, duration_ms, exit_code, http_status, error, result_truncated, result_bytes, created_at. Indexes on (session_id+created_at), (engine+tool_name+created_at), and partial index on (error) WHERE error IS NOT NULL.engines/sqliteAuditLogger.ts(~60L) — implements V1'sAuditLogger. Prepared INSERT reused across calls. Falls back to"unknown"if scope is missing (defensive — should never happen with the agent-loop wiring).engines/audit.ts— extendsAuditRowwith optionalsessionIdandengineName. V1 callers still compile (fields are optional).engines/agentLoop.ts— one-line change at thebuildAuditRowcall site: passestoolContext.sessionIdand.engineName(which were already there from V1, just not flowing through).gateway/server.ts— constructsSqliteAuditLoggeronce when at least one HTTP engine is configured. Passes the same instance to bothOllamaEngineandOpenAIEngine. Claude-only deployments don't pay for the import.Explicit safety guarantees
schema has no 'content', 'stdout', 'stderr', or 'body' columns) and a sentinel test (THIS_IS_FILE_BODY_THAT_MUST_NOT_LEAKdoes not appear in serialized row) protect this.safeAudit()(from V1 Phase 6a) catches and forwards tologger.warn. A dedicated test (record() throws when the DB has been closed mid-session) confirms the throw propagates sosafeAudithas something to catch.SqliteAuditLogger's constructor callsprepare()which throwsno such tableif migration 0007 hasn't been applied. Operator sees fail-fast, not a silent skip.Test plan
pnpm test722/722 (up from 707 in V1)AuditRowwideningOut of scope (queued as hristo2612#20)
🤖 Generated with Claude Code