fix(mcp): correct fiscal_periods column ref + remove agent auto-commit (#394)
* fix(mcp): correct fiscal_periods column ref + remove agent auto-commit The MCP `gnubok_list_fiscal_periods` tool selected a non-existent `fiscal_periods.status` column, causing `column fiscal_periods.status does not exist` errors when agents called it. Fixed by selecting the real columns (`is_closed`, `locked_at`, `closed_at`, `opening_balances_set`) and deriving `status` in code. Also removes the agent auto-commit feature (settings card, gating logic, DB columns, tests). In its current shape only `create_customer` was auto-commitable, so the toggle changed nothing meaningful in practice while implying a level of agent autonomy that wasn't actually granted. The risk-tier infrastructure on `pending_operations` (actor model, risk_level) is kept since it's still used by the /pending UI filters. Migration `20260505120000_drop_agent_auto_commit.sql` drops: - pending_operations.auto_commit_eligible - pending_operations.auto_committed_at - company_settings.agent_auto_commit_enabled - company_settings.agent_auto_commit_max_amount Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): explicitly pick fields in gnubok_list_fiscal_periods response Greptile flagged that `...p` spreads raw DB columns (`is_closed`, `locked_at`, `closed_at`) into the tool response alongside the derived `status`. Drop the spread for an explicit field list so the tool contract is the derived status only — agents don't need to reason about raw columns, and future SELECT additions won't silently leak. Also drops `closed_at` from the SELECT since it wasn't read. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
5c52f24a49
commit
b05f0e59b9
@@ -396,9 +396,6 @@ export const UpdateSettingsSchema = z.object({
|
||||
invoice_credit_terms_text: z.string().nullable().optional(),
|
||||
// AI agent flow
|
||||
ai_flow_enabled: z.boolean().optional(),
|
||||
// Agent auto-commit (low-risk ops staged by trusted agents)
|
||||
agent_auto_commit_enabled: z.boolean().optional(),
|
||||
agent_auto_commit_max_amount: z.number().nullable().optional(),
|
||||
}).refine(
|
||||
(data) => {
|
||||
// BFL 3 kap.: Enskild firma must have fiscal year starting January
|
||||
|
||||
+6
-54
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* pg-real smoke tests for the four migrations introduced by the
|
||||
* AI-native streams (actor model, auto-commit, expanded op types, idempotency).
|
||||
* pg-real smoke tests for the migrations introduced by the AI-native streams
|
||||
* (actor model, expanded op types, idempotency).
|
||||
*
|
||||
* These don't replicate the unit-test coverage — they prove the schema and
|
||||
* constraints behave as the application code assumes when running against a
|
||||
@@ -10,7 +10,7 @@ import { describe, expect, it } from 'vitest'
|
||||
import { getPool } from '@/tests/pg/setup'
|
||||
import { seedCompany } from '@/tests/pg/fixtures'
|
||||
|
||||
describe('pending_operations: actor model + risk + auto-commit columns', () => {
|
||||
describe('pending_operations: actor model + risk columns', () => {
|
||||
it('accepts the expanded actor_type and risk_level enums', async () => {
|
||||
const { userId, companyId } = await seedCompany()
|
||||
const pool = getPool()
|
||||
@@ -19,21 +19,19 @@ describe('pending_operations: actor model + risk + auto-commit columns', () => {
|
||||
id: string
|
||||
actor_type: string
|
||||
risk_level: string
|
||||
auto_commit_eligible: boolean
|
||||
}>(
|
||||
`INSERT INTO public.pending_operations (
|
||||
user_id, company_id, operation_type, title, params, preview_data,
|
||||
actor_type, actor_id, actor_label, risk_level, auto_commit_eligible
|
||||
actor_type, actor_id, actor_label, risk_level
|
||||
) VALUES ($1, $2, 'create_customer', 'pg-real test', '{}', '{}',
|
||||
'api_key', NULL, 'Claude Desktop', 'low', true)
|
||||
RETURNING id, actor_type, risk_level, auto_commit_eligible`,
|
||||
'api_key', NULL, 'Claude Desktop', 'low')
|
||||
RETURNING id, actor_type, risk_level`,
|
||||
[userId, companyId],
|
||||
)
|
||||
|
||||
expect(result.rows[0]).toMatchObject({
|
||||
actor_type: 'api_key',
|
||||
risk_level: 'low',
|
||||
auto_commit_eligible: true,
|
||||
})
|
||||
})
|
||||
|
||||
@@ -61,20 +59,6 @@ describe('pending_operations: actor model + risk + auto-commit columns', () => {
|
||||
).rejects.toThrow(/check constraint|risk_level/i)
|
||||
})
|
||||
|
||||
it('blocks auto_committed_at when status is still pending', async () => {
|
||||
const { userId, companyId } = await seedCompany()
|
||||
await expect(
|
||||
getPool().query(
|
||||
`INSERT INTO public.pending_operations (
|
||||
user_id, company_id, operation_type, title, params, preview_data,
|
||||
actor_type, risk_level, auto_committed_at
|
||||
) VALUES ($1, $2, 'create_customer', 'x', '{}', '{}',
|
||||
'api_key', 'low', now())`,
|
||||
[userId, companyId],
|
||||
),
|
||||
).rejects.toThrow(/pending_ops_auto_commit_status|check constraint/i)
|
||||
})
|
||||
|
||||
it('accepts the expanded operation_type enum (close_period, run_year_end, …)', async () => {
|
||||
const { userId, companyId } = await seedCompany()
|
||||
const expandedTypes = [
|
||||
@@ -111,38 +95,6 @@ describe('audit_log: actor_type + actor_label columns', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('company_settings: auto_commit columns', () => {
|
||||
it('exposes agent_auto_commit_enabled (default false) and agent_auto_commit_max_amount (NULL)', async () => {
|
||||
const { companyId } = await seedCompany()
|
||||
|
||||
const settings = await getPool().query<{
|
||||
enabled: boolean
|
||||
max_amount: string | null
|
||||
}>(
|
||||
`SELECT agent_auto_commit_enabled AS enabled,
|
||||
agent_auto_commit_max_amount AS max_amount
|
||||
FROM public.company_settings
|
||||
WHERE company_id = $1`,
|
||||
[companyId],
|
||||
)
|
||||
|
||||
// company_settings may or may not have a default row; if it doesn't, the
|
||||
// columns still exist on the table and we can inspect the catalog.
|
||||
if (settings.rows.length === 0) {
|
||||
const catalog = await getPool().query<{ name: string }>(
|
||||
`SELECT column_name AS name FROM information_schema.columns
|
||||
WHERE table_schema = 'public' AND table_name = 'company_settings'
|
||||
AND column_name IN ('agent_auto_commit_enabled', 'agent_auto_commit_max_amount')`,
|
||||
)
|
||||
expect(catalog.rowCount).toBe(2)
|
||||
return
|
||||
}
|
||||
|
||||
expect(settings.rows[0]?.enabled).toBe(false)
|
||||
expect(settings.rows[0]?.max_amount).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('idempotency_keys table', () => {
|
||||
it('enforces unique (user_id, key) and 24h default expiry', async () => {
|
||||
const { userId, companyId } = await seedCompany()
|
||||
@@ -60,8 +60,6 @@ function makePendingOp(overrides: Partial<PendingOperation>): PendingOperation {
|
||||
actor_id: null,
|
||||
actor_label: null,
|
||||
risk_level: 'high',
|
||||
auto_commit_eligible: false,
|
||||
auto_committed_at: null,
|
||||
created_at: '2026-05-03T00:00:00Z',
|
||||
resolved_at: null,
|
||||
updated_at: '2026-05-03T00:00:00Z',
|
||||
|
||||
@@ -1,154 +0,0 @@
|
||||
import { describe, it, expect, vi } from 'vitest'
|
||||
import { shouldAutoCommit } from '../should-auto-commit'
|
||||
|
||||
function mockSettingsClient(settings: { agent_auto_commit_enabled?: boolean; agent_auto_commit_max_amount?: number | null } | null) {
|
||||
return {
|
||||
from: vi.fn().mockReturnValue({
|
||||
select: vi.fn().mockReturnValue({
|
||||
eq: vi.fn().mockReturnValue({
|
||||
maybeSingle: vi.fn().mockResolvedValue(
|
||||
settings === null
|
||||
? { data: null, error: null }
|
||||
: { data: settings, error: null }
|
||||
),
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
} as never
|
||||
}
|
||||
|
||||
describe('shouldAutoCommit', () => {
|
||||
it('rejects high-risk ops regardless of any other config', async () => {
|
||||
const supabase = mockSettingsClient({ agent_auto_commit_enabled: true })
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'send_invoice',
|
||||
actorType: 'api_key',
|
||||
})
|
||||
expect(decision.eligible).toBe(false)
|
||||
expect(decision.risk_level).toBe('high')
|
||||
expect(decision.reason).toContain('high-risk')
|
||||
})
|
||||
|
||||
it('rejects user actors (they approve via UI)', async () => {
|
||||
const supabase = mockSettingsClient({ agent_auto_commit_enabled: true })
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'create_customer',
|
||||
actorType: 'user',
|
||||
})
|
||||
expect(decision.eligible).toBe(false)
|
||||
expect(decision.reason).toContain('approve via the UI')
|
||||
})
|
||||
|
||||
it('rejects when company has not opted in', async () => {
|
||||
const supabase = mockSettingsClient({ agent_auto_commit_enabled: false })
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'create_customer',
|
||||
actorType: 'api_key',
|
||||
})
|
||||
expect(decision.eligible).toBe(false)
|
||||
expect(decision.reason).toContain('not opted in')
|
||||
})
|
||||
|
||||
it('rejects medium-risk ops in current phase', async () => {
|
||||
const supabase = mockSettingsClient({ agent_auto_commit_enabled: true })
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'categorize_transaction',
|
||||
actorType: 'api_key',
|
||||
})
|
||||
expect(decision.eligible).toBe(false)
|
||||
expect(decision.reason).toContain('low-risk')
|
||||
})
|
||||
|
||||
it('approves low-risk ops from api_key with company opt-in', async () => {
|
||||
const supabase = mockSettingsClient({ agent_auto_commit_enabled: true })
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'create_customer',
|
||||
actorType: 'api_key',
|
||||
})
|
||||
expect(decision.eligible).toBe(true)
|
||||
expect(decision.risk_level).toBe('low')
|
||||
})
|
||||
|
||||
it('blocks low-risk op when amount exceeds threshold', async () => {
|
||||
const supabase = mockSettingsClient({
|
||||
agent_auto_commit_enabled: true,
|
||||
agent_auto_commit_max_amount: 1000,
|
||||
})
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'create_customer',
|
||||
actorType: 'api_key',
|
||||
amount: 5000,
|
||||
})
|
||||
expect(decision.eligible).toBe(false)
|
||||
expect(decision.reason).toContain('exceeds')
|
||||
})
|
||||
|
||||
it('approves low-risk op when amount within threshold', async () => {
|
||||
const supabase = mockSettingsClient({
|
||||
agent_auto_commit_enabled: true,
|
||||
agent_auto_commit_max_amount: 1000,
|
||||
})
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'create_customer',
|
||||
actorType: 'api_key',
|
||||
amount: 500,
|
||||
})
|
||||
expect(decision.eligible).toBe(true)
|
||||
})
|
||||
|
||||
it('approves low-risk op when amount missing (no threshold check applies)', async () => {
|
||||
const supabase = mockSettingsClient({
|
||||
agent_auto_commit_enabled: true,
|
||||
agent_auto_commit_max_amount: 1000,
|
||||
})
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'create_customer',
|
||||
actorType: 'api_key',
|
||||
// amount intentionally undefined
|
||||
})
|
||||
expect(decision.eligible).toBe(true)
|
||||
})
|
||||
|
||||
it('cron actors auto-commit non-high-risk without DB lookup', async () => {
|
||||
const supabase = mockSettingsClient(null)
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'create_customer',
|
||||
actorType: 'cron',
|
||||
})
|
||||
expect(decision.eligible).toBe(true)
|
||||
expect(decision.reason).toContain('Cron')
|
||||
})
|
||||
|
||||
it('cron actors still rejected for high-risk ops', async () => {
|
||||
const supabase = mockSettingsClient(null)
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'send_invoice',
|
||||
actorType: 'cron',
|
||||
})
|
||||
expect(decision.eligible).toBe(false)
|
||||
})
|
||||
|
||||
it('falls back to human approval when settings missing', async () => {
|
||||
const supabase = mockSettingsClient(null)
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'create_customer',
|
||||
actorType: 'api_key',
|
||||
})
|
||||
expect(decision.eligible).toBe(false)
|
||||
expect(decision.reason).toContain('Could not read company settings')
|
||||
})
|
||||
|
||||
it('treats negative amounts (refunds) by absolute value', async () => {
|
||||
const supabase = mockSettingsClient({
|
||||
agent_auto_commit_enabled: true,
|
||||
agent_auto_commit_max_amount: 1000,
|
||||
})
|
||||
const decision = await shouldAutoCommit(supabase, 'company-1', {
|
||||
operationType: 'create_customer',
|
||||
actorType: 'api_key',
|
||||
amount: -5000,
|
||||
})
|
||||
expect(decision.eligible).toBe(false)
|
||||
expect(decision.reason).toContain('exceeds')
|
||||
})
|
||||
})
|
||||
@@ -79,12 +79,6 @@ export interface CommitResult {
|
||||
export interface CommitOptions {
|
||||
/** Email address used as cc on send_invoice (typically the human user's email). */
|
||||
userEmail?: string
|
||||
/**
|
||||
* When true, the op was auto-committed by a trusted agent (no human in the
|
||||
* loop). The status row is updated with `auto_committed_at` so the UI can
|
||||
* surface this on /pending and in audit reports.
|
||||
*/
|
||||
isAutoCommit?: boolean
|
||||
}
|
||||
|
||||
// ── Helper: ensure fiscal period covers the date ──────────────────
|
||||
@@ -1574,18 +1568,13 @@ export async function commitPendingOperation(
|
||||
}
|
||||
|
||||
const now = new Date().toISOString()
|
||||
const update: Record<string, unknown> = {
|
||||
status: 'committed',
|
||||
resolved_at: now,
|
||||
result_data: result.data || {},
|
||||
}
|
||||
if (opts.isAutoCommit) {
|
||||
update.auto_committed_at = now
|
||||
}
|
||||
|
||||
await supabase
|
||||
.from('pending_operations')
|
||||
.update(update)
|
||||
.update({
|
||||
status: 'committed',
|
||||
resolved_at: now,
|
||||
result_data: result.data || {},
|
||||
})
|
||||
.eq('id', pendingOp.id)
|
||||
|
||||
return {
|
||||
|
||||
@@ -1,139 +0,0 @@
|
||||
/**
|
||||
* Decide whether a freshly-staged pending_operation should be auto-committed
|
||||
* by a trusted agent without human approval.
|
||||
*
|
||||
* Defense-in-depth: high-risk operations (period close, year-end, send_invoice,
|
||||
* etc.) are NEVER auto-committed regardless of company settings or actor
|
||||
* trust — that gate lives in risk-tiers.ts and is checked here before any
|
||||
* config lookup.
|
||||
*
|
||||
* Trust hierarchy:
|
||||
* - 'user' actors are humans clicking in the UI; auto-commit doesn't apply
|
||||
* (the click IS the approval)
|
||||
* - 'api_key' / 'mcp_oauth' actors are agents; eligible for auto-commit if
|
||||
* the company opts in and the op is low-risk
|
||||
* - 'cron' actors are system tasks; always auto-commit (they have no
|
||||
* human in the loop by design)
|
||||
*
|
||||
* Monetary threshold:
|
||||
* When `agent_auto_commit_max_amount` is set, any low-risk op with a
|
||||
* preview/payload amount above the threshold falls back to human approval.
|
||||
* The amount is read from the preview_data — callers should put it under
|
||||
* `amount` or `total` for the gate to find it. Missing amount → not blocked
|
||||
* by the threshold (safe for ops like create_customer where there's no
|
||||
* single dollar value).
|
||||
*/
|
||||
import type { SupabaseClient } from '@supabase/supabase-js'
|
||||
import { isHighRisk, getRiskLevel, type RiskLevel } from './risk-tiers'
|
||||
|
||||
export type AutoCommitActorType = 'user' | 'api_key' | 'mcp_oauth' | 'cron'
|
||||
|
||||
export interface AutoCommitInput {
|
||||
operationType: string
|
||||
actorType: AutoCommitActorType
|
||||
/** Optional monetary amount to check against agent_auto_commit_max_amount. */
|
||||
amount?: number | null
|
||||
}
|
||||
|
||||
export interface AutoCommitDecision {
|
||||
eligible: boolean
|
||||
reason: string
|
||||
risk_level: RiskLevel
|
||||
}
|
||||
|
||||
/**
|
||||
* Cheap pure-logic check that doesn't hit the DB. Used to short-circuit
|
||||
* obvious "no" cases before reading company_settings.
|
||||
*/
|
||||
function precheck(input: AutoCommitInput): AutoCommitDecision | null {
|
||||
const risk = getRiskLevel(input.operationType)
|
||||
|
||||
if (isHighRisk(input.operationType)) {
|
||||
return {
|
||||
eligible: false,
|
||||
reason: `Operation "${input.operationType}" is high-risk and never auto-committed.`,
|
||||
risk_level: risk,
|
||||
}
|
||||
}
|
||||
|
||||
if (input.actorType === 'user') {
|
||||
return {
|
||||
eligible: false,
|
||||
reason: 'User actors approve via the UI; auto-commit does not apply.',
|
||||
risk_level: risk,
|
||||
}
|
||||
}
|
||||
|
||||
// Cron actors auto-commit non-high-risk regardless of company config.
|
||||
// Resolved here so we don't read company_settings unnecessarily.
|
||||
if (input.actorType === 'cron') {
|
||||
return {
|
||||
eligible: true,
|
||||
reason: 'Cron actor: auto-commit allowed for non-high-risk ops.',
|
||||
risk_level: risk,
|
||||
}
|
||||
}
|
||||
|
||||
// For api_key/mcp_oauth: only low-risk is auto-committable in this phase.
|
||||
// Reject medium-risk before the company_settings lookup so callers don't pay
|
||||
// for a DB read that can't succeed.
|
||||
if (risk !== 'low') {
|
||||
return {
|
||||
eligible: false,
|
||||
reason: 'Only low-risk operations are auto-committable in the current phase.',
|
||||
risk_level: risk,
|
||||
}
|
||||
}
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
export async function shouldAutoCommit(
|
||||
supabase: SupabaseClient,
|
||||
companyId: string,
|
||||
input: AutoCommitInput
|
||||
): Promise<AutoCommitDecision> {
|
||||
const pre = precheck(input)
|
||||
if (pre) return pre
|
||||
|
||||
const risk = getRiskLevel(input.operationType)
|
||||
|
||||
// api_key / mcp_oauth + low-risk: gated by company opt-in and threshold.
|
||||
const { data: settings, error } = await supabase
|
||||
.from('company_settings')
|
||||
.select('agent_auto_commit_enabled, agent_auto_commit_max_amount')
|
||||
.eq('company_id', companyId)
|
||||
.maybeSingle()
|
||||
|
||||
if (error || !settings) {
|
||||
return {
|
||||
eligible: false,
|
||||
reason: 'Could not read company settings; defaulting to human approval.',
|
||||
risk_level: risk,
|
||||
}
|
||||
}
|
||||
|
||||
if (!settings.agent_auto_commit_enabled) {
|
||||
return {
|
||||
eligible: false,
|
||||
reason: 'Company has not opted in to agent auto-commit.',
|
||||
risk_level: risk,
|
||||
}
|
||||
}
|
||||
|
||||
const max = settings.agent_auto_commit_max_amount
|
||||
const amount = input.amount
|
||||
if (max != null && amount != null && Math.abs(amount) > Number(max)) {
|
||||
return {
|
||||
eligible: false,
|
||||
reason: `Amount ${amount} exceeds company auto-commit threshold ${max}.`,
|
||||
risk_level: risk,
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
eligible: true,
|
||||
reason: 'Low-risk op, trusted actor, company opted in, amount within limit.',
|
||||
risk_level: risk,
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user