From 8f7034ec0074d991d033273250a5d615cbb15d5d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 15 Oct 2026 16:19:56 -0700 Subject: [PATCH 1/7] fix(billing): answer an in-flight retry at once, and tell Chat what provenance withheld A cost callback retried while an earlier attempt of the same request was still committing (a commit can wait on a synchronous standby while holding the request's ledger lock) waited out lock_timeout on a pooled connection and failed with a 500. The ledger lock is now taken without waiting: a held lock answers 503 BILLING_REQUEST_IN_FLIGHT (Retry-After) at once, and the caller resends later; the monotonic top-up applies the higher cumulative once. Provenance refusals are policy, not faults: each gate admission now reports its refusals as one warning with counts and a bounded sample instead of one error per item, and the table envelope's unknown-row line is a warning. Chat's CLI result now notes how many items of each kind a gate withheld, on stderr, so the model can tell a withheld item from an absent one. Callers outside Chat never classify content and see no change. Also corrects two comments: the worker's callback timeout is 15 s, and no transaction setting bounds a commit waiting on a standby. --- .../app/api/billing/update-cost/route.test.ts | 37 +++++++++++ apps/sim/app/api/billing/update-cost/route.ts | 28 +++++++- .../lib/billing/core/usage-log.integration.ts | 25 +++++-- apps/sim/lib/billing/core/usage-log.ts | 40 +++++++---- .../core/application/content-delivery.test.ts | 28 ++++++++ .../lib/core/application/content-delivery.ts | 29 ++++++++ apps/sim/lib/core/application/index.ts | 1 + ...urable-secret-provenance-telemetry.test.ts | 29 +++++++- .../durable-secret-provenance-telemetry.ts | 37 ++++++++++- .../agent-cli/file-provenance.test.ts | 10 ++- apps/sim/lib/mothership/agent-cli/index.ts | 14 ++++ .../content-delivery/registry-gate.test.ts | 19 +++++- .../content-delivery/registry-gate.ts | 17 +++-- .../content-delivery/session-gate.test.ts | 17 ++++- .../content-delivery/session-gate.ts | 8 ++- .../mothership/content-delivery/telemetry.ts | 66 +++++++++++++++---- apps/sim/lib/mothership/generated/billing.ts | 2 + apps/sim/lib/table/rows/secret-provenance.ts | 3 +- .../src/mocks/billing-usage-log.mock.ts | 12 +++- packages/testing/src/mocks/index.ts | 1 + 20 files changed, 372 insertions(+), 51 deletions(-) diff --git a/apps/sim/app/api/billing/update-cost/route.test.ts b/apps/sim/app/api/billing/update-cost/route.test.ts index 29f86b66aea..826e1bfc737 100644 --- a/apps/sim/app/api/billing/update-cost/route.test.ts +++ b/apps/sim/app/api/billing/update-cost/route.test.ts @@ -9,6 +9,7 @@ import { billingPlanMock, billingPlanMockFns } from '@sim/testing/mocks/billing- import { billingUsageLogMock, billingUsageLogMockFns, + MockCumulativeUsageInFlightError, } from '@sim/testing/mocks/billing-usage-log.mock' import { billingUsageMonitorMock, @@ -18,6 +19,7 @@ import { copilotHttpMock, copilotHttpMockFns } from '@sim/testing/mocks/copilot- import { mothershipOtelMock } from '@sim/testing/mocks/mothership-otel.mock' import { sleep } from '@sim/utils/helpers' import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { BillingInFlightCode } from '@/lib/mothership/generated/billing' const { mockCheckAndBillOverageThreshold, @@ -748,6 +750,41 @@ describe('POST /api/billing/update-cost — workspaceId attribution', () => { }) }) + it('answers a retryable in-flight 503 while another attempt of the request is recorded', async () => { + const billingRequestId = '0190c03f-9f7d-4b79-8b58-e7f779fd29e1' + mockRecordCumulativeUsage.mockRejectedValueOnce( + new MockCumulativeUsageInFlightError(`update-cost:${billingRequestId}`) + ) + + const res = await POST( + createMockRequest( + 'POST', + { + userId: 'user-1', + cost: 0.5, + model: 'claude-opus-4.8', + source: 'workspace-chat', + idempotencyKey: billingRequestId, + }, + { + 'x-api-key': 'internal', + 'x-sim-billing-protocol': 'direct-v1', + 'x-sim-billing-request-id': billingRequestId, + 'x-sim-billing-account-decision': 'serialized-account-decision', + } + ) + ) + + expect(res.status).toBe(503) + expect(res.headers.get('retry-after')).toBe('5') + await expect(res.json()).resolves.toMatchObject({ + success: false, + code: BillingInFlightCode, + retryable: true, + }) + expect(mockCheckAndBillPayerOverageThreshold).not.toHaveBeenCalled() + }) + it('retries modern settlement on a duplicate cumulative callback before returning 409', async () => { const billingRequestId = '0190c03f-9f7d-4b79-8b58-e7f779fd29e1' mockRecordCumulativeUsage diff --git a/apps/sim/app/api/billing/update-cost/route.ts b/apps/sim/app/api/billing/update-cost/route.ts index 3c3668cb732..8857cec569e 100644 --- a/apps/sim/app/api/billing/update-cost/route.ts +++ b/apps/sim/app/api/billing/update-cost/route.ts @@ -30,6 +30,7 @@ import { import { type CumulativeUsageContextField, CumulativeUsageContextMismatchError, + CumulativeUsageInFlightError, CumulativeUsagePeriodClosedError, recordCumulativeUsage, } from '@/lib/billing/core/usage-log' @@ -43,6 +44,7 @@ import { isBillingEnabled, isHosted } from '@/lib/core/config/env-flags' import { withinDeadline } from '@/lib/core/utils/deadline' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { BillingInFlightCode } from '@/lib/mothership/generated/billing' import { BILLING_CALLBACK_OUTCOME } from '@/lib/mothership/generated/billing-protocol-v1' import { BillingRouteOutcome } from '@/lib/mothership/generated/trace-attribute-values-v1' import { TraceAttr } from '@/lib/mothership/generated/trace-attributes-v1' @@ -53,7 +55,7 @@ import { withIncomingGoSpan } from '@/lib/mothership/request/otel' const logger = createLogger('BillingUpdateCostAPI') /** * How long a cost callback waits on the payer's standing. The worker gives up on the whole - * callback after 5 s, and a cold gate read can wait on the ledger far longer; past this the + * callback after 15 s, and a cold gate read can wait on the ledger far longer; past this the * callback answers not-exceeded. The abandoned read keeps running and caches its admission, and * the next step or re-check reads a refusal again. */ @@ -510,6 +512,30 @@ async function updateCostInner(req: NextRequest, span: Span): Promise ({ import { CumulativeUsageContextMismatchError, + CumulativeUsageInFlightError, CumulativeUsagePeriodClosedError, getBillingPeriodUsageCost, getBillingPeriodUsageCostByUser, @@ -92,6 +93,18 @@ function usage(cost: number, eventKey = 'update-cost:shared-request'): RecordCum } } +/** Resends a callback refused as in flight, the way the worker's replay does, until it is recorded. */ +async function untilRecorded(params: RecordCumulativeUsageParams) { + for (let attempt = 0; ; attempt++) { + try { + return await recordCumulativeUsage(params) + } catch (error) { + if (!(error instanceof CumulativeUsageInFlightError) || attempt >= 200) throw error + await sleep(50) + } + } +} + async function ledgerRows() { return connection<{ event_key: string; cost: string }[]>` select event_key, cost from usage_log order by event_key @@ -199,13 +212,15 @@ describe('Cumulative billing with PostgreSQL', () => { } ) - it('reproduces a retry timing out behind an idle holder when only lock waits are bounded', async () => { + it('refuses a retry behind a holder of the same request at once, without waiting on its lock', async () => { const pause = pauseNextTransaction(true) const holder = recordCumulativeUsage(usage(0.4)) try { await pause.reached.promise + const startedAt = performance.now() const failure = await recordCumulativeUsage(usage(0.8)).catch((error: unknown) => error) - expect(getPostgresErrorCode(failure)).toBe('55P03') + expect(failure).toBeInstanceOf(CumulativeUsageInFlightError) + expect(performance.now() - startedAt).toBeLessThan(1_000) expect(await ledgerRows()).toEqual([]) } finally { pause.release.resolve() @@ -223,8 +238,8 @@ describe('Cumulative billing with PostgreSQL', () => { try { await pause.reached.promise const failure = await recordCumulativeUsage(usage(0.8)).catch((error: unknown) => error) - expect(getPostgresErrorCode(failure)).toBe('55P03') - const recovered = await recordCumulativeUsage(usage(0.8)) + expect(failure).toBeInstanceOf(CumulativeUsageInFlightError) + const recovered = await untilRecorded(usage(0.8)) expect(recovered.billed).toBe(true) expect(recovered.delta).toBeCloseTo(0.8 - initial, 9) expect(recovered.total).toBe(0.8) @@ -249,7 +264,7 @@ describe('Cumulative billing with PostgreSQL', () => { it('converges concurrent out-of-order callbacks and independent events to their exact totals', async () => { const costs = [0.4, 0.1, 0.8, 0.3, 0.8, 0.6] - const results = await Promise.all(costs.map((cost) => recordCumulativeUsage(usage(cost)))) + const results = await Promise.all(costs.map((cost) => untilRecorded(usage(cost)))) expect(results.reduce((total, result) => total + result.delta, 0)).toBeCloseTo(0.8, 9) await Promise.all( Array.from({ length: 32 }, (_, index) => diff --git a/apps/sim/lib/billing/core/usage-log.ts b/apps/sim/lib/billing/core/usage-log.ts index 78bfb51ac06..44004996ad1 100644 --- a/apps/sim/lib/billing/core/usage-log.ts +++ b/apps/sim/lib/billing/core/usage-log.ts @@ -30,7 +30,7 @@ import { isOrgScopedSubscription } from '@/lib/billing/subscriptions/utils' import type { InternalUsageLogSource } from '@/lib/billing/usage-sources' import { asOrchestrationError, OrchestrationError } from '@/lib/core/orchestration/types' import { HttpError } from '@/lib/core/utils/http-error' -import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks' +import { tryAcquireAdvisoryXactLock } from '@/lib/db/advisory-locks' import type { DbClient, DbOrTx } from '@/lib/db/types' const logger = createLogger('UsageLog') @@ -700,6 +700,18 @@ export class CumulativeUsagePeriodClosedError extends Error { } } +/** + * Another attempt of the same request holds its ledger lock, so this one cannot know whether that + * attempt's charge will land. Waiting would only hold a connection for an answer the caller gets by + * retrying later: the monotonic top-up applies whichever cumulative arrives once. + */ +export class CumulativeUsageInFlightError extends Error { + constructor(readonly eventKey: string) { + super(`Cumulative usage event "${eventKey}" is already being recorded`) + this.name = 'CumulativeUsageInFlightError' + } +} + interface CumulativeUsageLedgerBinding { userId: string workspaceId: string | null @@ -755,11 +767,13 @@ function assertCumulativeUsageLedgerBinding( } /** - * PostgreSQL 17+ bounds the entire transaction below the callback's five-second - * deadline. Older supported servers instead bound each idle interval between - * statements, alongside the per-statement budget. Both policies release an idle - * lock holder without waiting for its application process to resume; only the - * newer policy also limits total elapsed transaction time. + * PostgreSQL 17+ bounds the transaction's statements and the idle time between + * them; older supported servers bound each idle interval instead, alongside the + * per-statement budget. Both release an idle lock holder without waiting for its + * application process to resume. Neither bounds COMMIT: a commit waiting for a + * synchronous standby keeps its locks, including the request's ledger lock, until + * the standby confirms, however long that takes. `lock_timeout` bounds the row + * locks taken under the ledger lock; the ledger lock itself is never waited for. */ const CUMULATIVE_FLUSH_TRANSACTION_TIMEOUT_MS = 4_000 const CUMULATIVE_FLUSH_STATEMENT_TIMEOUT_MS = 3_500 @@ -778,10 +792,12 @@ type CumulativeUsageStage = 'pool' | 'configure' | 'lock' | 'read' | 'write' | ' * (no two flushes can both believe they are first and clobber each other). * An existing row must match the incoming actor, workspace, payer, and billing * period before either a duplicate no-op or a top-up is accepted. - * The billing context is resolved BEFORE the transaction and the lock wait is - * bounded by `lock_timeout`. A server-enforced transaction deadline, or idle - * transaction deadline on older PostgreSQL, releases a stalled holder. The - * critical section uses one SELECT plus one INSERT/UPDATE on a single connection. + * The billing context is resolved BEFORE the transaction. The lock is taken + * without waiting: a held lock means another attempt of the same request is + * still being recorded (possibly committing behind a synchronous standby), and + * this attempt throws {@link CumulativeUsageInFlightError} instead of holding a + * connection while it waits. The critical section uses one SELECT plus one + * INSERT/UPDATE on a single connection. * * Because every leg flushes its cumulative and this converges to the max, * there is no under-billing if the request recovers after a partial flush, no @@ -841,7 +857,9 @@ export async function recordCumulativeUsage( set_config('lock_timeout', ${`${CUMULATIVE_FLUSH_LOCK_TIMEOUT_MS}ms`}, true) `) enterStage('lock') - await acquireAdvisoryXactLock(tx, 'usage_log_event', eventKey) + if (!(await tryAcquireAdvisoryXactLock(tx, 'usage_log_event', eventKey))) { + throw new CumulativeUsageInFlightError(eventKey) + } enterStage('read') const rows = await tx diff --git a/apps/sim/lib/core/application/content-delivery.test.ts b/apps/sim/lib/core/application/content-delivery.test.ts index 5cc74901232..a849c87cd94 100644 --- a/apps/sim/lib/core/application/content-delivery.test.ts +++ b/apps/sim/lib/core/application/content-delivery.test.ts @@ -12,6 +12,7 @@ import { ContentDeliveryUnobservedError, ContentRefusedError, type ContentSource, + countWithheldContent, mustClassifyContent, noteContentDeliveryOperation, observeContentDelivery, @@ -509,3 +510,30 @@ describe('observeContentOutsideModel', () => { expect(batches).toEqual([]) }) }) + +describe('countWithheldContent', () => { + it('counts every refused item by kind across the requests and use cases it ran', async () => { + const { gate } = recordingGate() + const row = (id: string): ContentSource => ({ ...source(id, 'unknown'), kind: 'table-row' }) + const { result, withheld } = await observeContentDelivery(gate, () => + countWithheldContent(async () => { + await trackContentDelivery(() => admitContent(CHAT, [row('a'), row('b'), source('clean')])) + const admission = await admitContent(CHAT, [source('file', 'unknown')]) + return admission.refused.length + }) + ) + expect(result).toBe(1) + expect([...withheld]).toEqual([ + ['table-row', 2], + ['workspace-file', 1], + ]) + }) + + it('counts nothing for a caller that never classifies', async () => { + const { gate } = recordingGate() + const { withheld } = await observeContentDelivery(gate, () => + countWithheldContent(() => admitContent(EXECUTOR, [source('file', 'unknown')])) + ) + expect(withheld.size).toBe(0) + }) +}) diff --git a/apps/sim/lib/core/application/content-delivery.ts b/apps/sim/lib/core/application/content-delivery.ts index 1d6a1bdec59..1ea2b03a563 100644 --- a/apps/sim/lib/core/application/content-delivery.ts +++ b/apps/sim/lib/core/application/content-delivery.ts @@ -73,6 +73,8 @@ interface ContentLedger { accounted: boolean /** Bounded aggregate of admitted provenance, for a surface that persists the delivered bytes. */ admitted: ContentProvenance[] + /** Only on a {@link countWithheldContent} ledger: the items refused inside it, by kind. */ + withheld?: Map /** A committed write's returned item was refused; the operation answers with a refusal. */ withheldAfterWrite?: ContentSourceKind /** The gate itself failed while this ledger was open, whatever the route then answered. */ @@ -303,6 +305,13 @@ export async function admitContent( } markAccounted() if (ledger) { + for (let current: ContentLedger | undefined = ledger; current; current = current.parent) { + const { withheld } = current + if (!withheld) continue + sources.forEach(({ kind }, index) => { + if (!verdicts[index]) withheld.set(kind, (withheld.get(kind) ?? 0) + 1) + }) + } const batch = mergeContentProvenance( (function* () { for (let index = 0; index < sources.length; index++) { @@ -459,6 +468,26 @@ export async function trackContentDelivery( } } +/** + * Runs `execute` and counts, by kind, the items a gate refused inside it, across every request and + * use case it ran. A refused item is left out of the result it was read in, so a surface reporting + * to Chat uses this to say what is missing rather than let it read as absent. + */ +export async function countWithheldContent( + execute: () => Promise +): Promise<{ result: T; withheld: ReadonlyMap }> { + const withheld = new Map() + const ledger: ContentLedger = { + parent: ledgers.getStore(), + sourcedOperationAdmitted: false, + accounted: false, + admitted: [], + withheld, + } + const result = await ledgers.run(ledger, execute) + return { result, withheld } +} + /** Route admission records that Chat was admitted to an operation, in the request's ledger. */ export function noteContentDeliveryOperation( operation: ApplicationOperation, diff --git a/apps/sim/lib/core/application/index.ts b/apps/sim/lib/core/application/index.ts index 5b91b1fd8f2..572006c2c77 100644 --- a/apps/sim/lib/core/application/index.ts +++ b/apps/sim/lib/core/application/index.ts @@ -74,6 +74,7 @@ export { type ContentSource, type ContentSourceKind, type ContentView, + countWithheldContent, isCopilotPrincipal, mustClassifyContent, noteContentDeliveryOperation, diff --git a/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts b/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts index 6d1d2c22870..526f1b0a7a4 100644 --- a/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts +++ b/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts @@ -2,6 +2,7 @@ import { getMockLogger } from '@sim/testing/mocks/logger.mock' import { describe, expect, it } from 'vitest' import { reportDurableSecretProvenanceRefusal, + reportDurableSecretProvenanceRefusals, reportDurableSecretProvenanceWrite, } from '@/lib/execution/durable-secret-provenance-telemetry' @@ -42,7 +43,7 @@ describe('durable secret provenance telemetry', () => { entries: [{ encryptedValue: 'private ciphertext' }], } reportDurableSecretProvenanceRefusal(report) - expect(logger.error).toHaveBeenCalledExactlyOnceWith( + expect(logger.warn).toHaveBeenCalledExactlyOnceWith( 'Refusing unavailable durable secret provenance', { surface: 'workspace-file', @@ -52,4 +53,30 @@ describe('durable secret provenance telemetry', () => { } ) }) + it('reports a batch of refusals as one line with counts and a bounded sample', () => { + logger.warn.mockClear() + const reports = Array.from({ length: 8 }, (_, index) => ({ + surface: 'table-row' as const, + cause: + index < 6 + ? ('table-row-provenance-unavailable' as const) + : ('table-row-opaque-secret-content' as const), + workspaceId: 'workspace-1', + resourceId: `row-${index}`, + })) + reportDurableSecretProvenanceRefusals(reports) + expect(logger.warn).toHaveBeenCalledExactlyOnceWith( + 'Refusing unavailable durable secret provenance', + { + refusedCount: 8, + causes: { + 'table-row:table-row-provenance-unavailable': 6, + 'table-row:table-row-opaque-secret-content': 2, + }, + workspaceId: 'workspace-1', + sampleResourceIds: ['row-0', 'row-1', 'row-2', 'row-3', 'row-4'], + } + ) + expect(logger.error).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts b/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts index 49c4c19d9f3..4c82f29e6f2 100644 --- a/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts +++ b/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts @@ -70,11 +70,14 @@ export function reportDurableSecretProvenanceWrite( }) } -/** Reports a durable read refusal without exposing content or secret values. */ +/** + * Reports a durable read refusal without exposing content or secret values. A refusal is policy + * applied to content that lacks provenance, not a fault, so it is a warning. + */ export function reportDurableSecretProvenanceRefusal( report: DurableSecretProvenanceRefusalReport ): void { - persistenceLogger.error('Refusing unavailable durable secret provenance', { + persistenceLogger.warn('Refusing unavailable durable secret provenance', { surface: report.surface, cause: report.cause, ...(report.workspaceId ? { workspaceId: report.workspaceId } : {}), @@ -82,6 +85,36 @@ export function reportDurableSecretProvenanceRefusal( }) } +/** Resource ids a batched refusal names, enough to find the items without listing a whole page. */ +const REFUSAL_SAMPLE_SIZE = 5 + +/** Reports every refusal of one read as a single line: counts per surface and cause, and a sample. */ +export function reportDurableSecretProvenanceRefusals( + reports: readonly DurableSecretProvenanceRefusalReport[] +): void { + const [first] = reports + if (!first) return + if (reports.length === 1) { + reportDurableSecretProvenanceRefusal(first) + return + } + const causes: Record = {} + for (const report of reports) { + const key = `${report.surface}:${report.cause}` + causes[key] = (causes[key] ?? 0) + 1 + } + const workspaceIds = new Set(reports.map((report) => report.workspaceId)) + const sampleResourceIds = reports + .flatMap((report) => (report.resourceId ? [report.resourceId] : [])) + .slice(0, REFUSAL_SAMPLE_SIZE) + persistenceLogger.warn('Refusing unavailable durable secret provenance', { + refusedCount: reports.length, + causes, + ...(workspaceIds.size === 1 && first.workspaceId ? { workspaceId: first.workspaceId } : {}), + ...(sampleResourceIds.length > 0 ? { sampleResourceIds } : {}), + }) +} + export interface DurableSecretProvenanceUnrecordedReport { surface: DurableSecretProvenanceSurface workspaceId?: string diff --git a/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts b/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts index a0638b0f092..9c087594faa 100644 --- a/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts +++ b/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts @@ -374,7 +374,7 @@ describe('file provenance at the actual CLI and model-result boundary', () => { }) it('refuses only the unknown item of a forwarded response, naming it, without blanking the invocation', async () => { - const refusals = getMockLogger('DurableSecretProvenancePersistence').error + const refusals = getMockLogger('DurableSecretProvenancePersistence').warn refusals.mockClear() const clean = fileSource('clean', { status: 'exact', entries: [] }) const tainted = fileSource('tainted', { status: 'unknown' }) @@ -552,7 +552,7 @@ describe('file provenance at the actual CLI and model-result boundary', () => { }) ) classifySequence('exact', 'unknown') - const refusals = getMockLogger('DurableSecretProvenancePersistence').error + const refusals = getMockLogger('DurableSecretProvenancePersistence').warn refusals.mockClear() const trace = registry() const result = await runCli(grepFiles('CANARY_FOR_LOCAL_TEST'), trace) @@ -569,6 +569,12 @@ describe('file provenance at the actual CLI and model-result boundary', () => { resourceId: tainted.id, }) ) + /** The model is told the file it cannot see exists, so it does not read the gap as absence. */ + expect(result.output).toMatchObject({ + stderr: expect.stringContaining( + 'Note: 1 file was withheld from this result because its secret provenance is unavailable.' + ), + }) }) describe('grep reshapes a file line only after redacting it', () => { diff --git a/apps/sim/lib/mothership/agent-cli/index.ts b/apps/sim/lib/mothership/agent-cli/index.ts index 4d7a7b525dd..bfc76115ffb 100644 --- a/apps/sim/lib/mothership/agent-cli/index.ts +++ b/apps/sim/lib/mothership/agent-cli/index.ts @@ -1,4 +1,5 @@ import { createEmbeddedClient, type EmbeddedCliIdentity } from 'sim/embed' +import { countWithheldContent } from '@/lib/core/application' import { withWorkspaceInvocationScope } from '@/lib/core/application/workspace-invocation-scope' import { getInternalApiBaseUrl } from '@/lib/core/utils/urls' import { curateBlockDetail } from '@/lib/mothership/agent-cli/curation' @@ -16,6 +17,7 @@ import { createCopilotChatPrincipal, createTrustedOrganizationCopilotPrincipal, } from '@/lib/mothership/auth/application-delegation' +import { describeWithheldContent } from '@/lib/mothership/content-delivery/telemetry' import type { AgentCliRawResult, AgentCliRequest } from '@/lib/mothership/generated/agent-cli' import type { ResourceChange } from '@/lib/mothership/generated/resources' import { TraceSpan } from '@/lib/mothership/generated/trace-spans-v1' @@ -38,6 +40,18 @@ export interface AgentCliExecutionContext extends Omit { + const { result, withheld } = await countWithheldContent(() => + executeUncountedAgentCliRequest(request, context) + ) + const note = describeWithheldContent(withheld) + if (!note) return result + return { ...result, stderr: result.stderr ? `${result.stderr}\n${note}` : note } +} + +async function executeUncountedAgentCliRequest( + request: AgentCliRequest, + context: AgentCliExecutionContext ): Promise { context.signal?.throwIfAborted() if (context.readOnly && !isReadOnlyCliRequest(request)) { diff --git a/apps/sim/lib/mothership/content-delivery/registry-gate.test.ts b/apps/sim/lib/mothership/content-delivery/registry-gate.test.ts index 2eea660c1d7..ce5ca212a7a 100644 --- a/apps/sim/lib/mothership/content-delivery/registry-gate.test.ts +++ b/apps/sim/lib/mothership/content-delivery/registry-gate.test.ts @@ -7,6 +7,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('@/lib/core/security/encryption', () => encryptionMock) const telemetry = vi.hoisted(() => ({ reportDurableSecretProvenanceRefusal: vi.fn(), + reportDurableSecretProvenanceRefusals: vi.fn(), reportDurableSecretProvenanceUnrecordedBatch: vi.fn(), })) vi.mock('@/lib/execution/durable-secret-provenance-telemetry', () => telemetry) @@ -122,12 +123,26 @@ describe('createRegistryContentGate', () => { const gate = createRegistryContentGate({ registry, actorUserId: 'user-1' }) expect(await gate.admit([item('archive', exact(SECRET), 'opaque')])).toEqual([false]) expect(registry.isPermanentlyIncomplete()).toBe(false) - expect(telemetry.reportDurableSecretProvenanceRefusal).toHaveBeenCalledWith( + expect(telemetry.reportDurableSecretProvenanceRefusals).toHaveBeenCalledWith([ expect.objectContaining({ resourceId: 'archive', cause: 'workspace-file-opaque-secret-content', - }) + }), + ]) + }) + + it('reports every item one admission refuses as one batch, not one line per item', async () => { + telemetry.reportDurableSecretProvenanceRefusals.mockClear() + const gate = createRegistryContentGate({ registry: newRegistry(), actorUserId: 'user-1' }) + const refused = Array.from({ length: 50 }, (_, index) => + item(`legacy-${index}`, { status: 'unknown' }) ) + const verdicts = await gate.admit([...refused, item('clean', exact())]) + expect(verdicts).toEqual([...refused.map(() => false), true]) + expect(telemetry.reportDurableSecretProvenanceRefusals).toHaveBeenCalledTimes(1) + const [reports] = telemetry.reportDurableSecretProvenanceRefusals.mock.calls[0] + expect(reports).toHaveLength(50) + expect(telemetry.reportDurableSecretProvenanceRefusal).not.toHaveBeenCalled() }) it('refuses an item holding secrets when there is no registry to redact with', async () => { diff --git a/apps/sim/lib/mothership/content-delivery/registry-gate.ts b/apps/sim/lib/mothership/content-delivery/registry-gate.ts index ba6bd14fa90..bdd66148c83 100644 --- a/apps/sim/lib/mothership/content-delivery/registry-gate.ts +++ b/apps/sim/lib/mothership/content-delivery/registry-gate.ts @@ -6,8 +6,8 @@ import { } from '@/lib/execution/durable-secret-provenance-telemetry' import { CONTENT_SURFACE, - type ContentRefusalReason, - reportContentRefusal, + type ContentRefusals, + createContentRefusals, } from '@/lib/mothership/content-delivery/telemetry' import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection' import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' @@ -33,12 +33,10 @@ export interface RegistryContentGateScope { * any entry, so a refused item cannot latch the registry for every other item. */ export function createRegistryContentGate(scope: RegistryContentGateScope): ContentDeliveryGate { - const refuse = (source: ContentSource, reason: ContentRefusalReason): false => - reportContentRefusal(source, reason, scope.workspaceId) - const decide = async ( source: ContentSource, - unrecorded: DurableSecretProvenanceUnrecordedReport[] + unrecorded: DurableSecretProvenanceUnrecordedReport[], + { refuse }: ContentRefusals ): Promise => { const { provenance } = source if (provenance.status === 'unknown') { @@ -89,8 +87,13 @@ export function createRegistryContentGate(scope: RegistryContentGateScope): Cont }, async admit(sources) { const unrecorded: DurableSecretProvenanceUnrecordedReport[] = [] + const refusals = createContentRefusals(scope.workspaceId) const verdicts: boolean[] = [] - for (const source of sources) verdicts.push(await decide(source, unrecorded)) + try { + for (const source of sources) verdicts.push(await decide(source, unrecorded, refusals)) + } finally { + refusals.report() + } reportDurableSecretProvenanceUnrecordedBatch(unrecorded) return verdicts }, diff --git a/apps/sim/lib/mothership/content-delivery/session-gate.test.ts b/apps/sim/lib/mothership/content-delivery/session-gate.test.ts index 164c2646bb7..6b6d74e8298 100644 --- a/apps/sim/lib/mothership/content-delivery/session-gate.test.ts +++ b/apps/sim/lib/mothership/content-delivery/session-gate.test.ts @@ -8,6 +8,7 @@ const history = vi.hoisted(() => ({ recordExistingSessionFileInput: vi.fn() })) vi.mock('@/lib/execution/remote-sandbox/session-file-provenance', () => history) const telemetry = vi.hoisted(() => ({ reportDurableSecretProvenanceRefusal: vi.fn(), + reportDurableSecretProvenanceRefusals: vi.fn(), reportDurableSecretProvenanceUnrecordedBatch: vi.fn(), })) vi.mock('@/lib/execution/durable-secret-provenance-telemetry', () => telemetry) @@ -44,9 +45,21 @@ describe('createSessionContentGate', () => { it('keeps an unknown item out of the machine and its history', async () => { expect(await gate().admit([item('tainted', { status: 'unknown' })])).toEqual([false]) expect(history.recordExistingSessionFileInput).not.toHaveBeenCalled() - expect(telemetry.reportDurableSecretProvenanceRefusal).toHaveBeenCalledWith( - expect.objectContaining({ resourceId: 'tainted' }) + expect(telemetry.reportDurableSecretProvenanceRefusals).toHaveBeenCalledWith([ + expect.objectContaining({ resourceId: 'tainted' }), + ]) + }) + + it('reports every item one admission refuses as one batch, not one line per item', async () => { + telemetry.reportDurableSecretProvenanceRefusals.mockClear() + const refused = Array.from({ length: 50 }, (_, index) => + item(`tainted-${index}`, { status: 'unknown' }) ) + expect(await gate().admit(refused)).toEqual(refused.map(() => false)) + expect(telemetry.reportDurableSecretProvenanceRefusals).toHaveBeenCalledTimes(1) + const [reports] = telemetry.reportDurableSecretProvenanceRefusals.mock.calls[0] + expect(reports).toHaveLength(50) + expect(telemetry.reportDurableSecretProvenanceRefusal).not.toHaveBeenCalled() }) it('admits secret-bearing bytes, even opaque, after merging their entries into the history', async () => { diff --git a/apps/sim/lib/mothership/content-delivery/session-gate.ts b/apps/sim/lib/mothership/content-delivery/session-gate.ts index a5fc9e387f3..5e7e89eafe7 100644 --- a/apps/sim/lib/mothership/content-delivery/session-gate.ts +++ b/apps/sim/lib/mothership/content-delivery/session-gate.ts @@ -7,7 +7,7 @@ import { } from '@/lib/execution/durable-secret-provenance' import { reportDurableSecretProvenanceUnrecordedBatch } from '@/lib/execution/durable-secret-provenance-telemetry' import { recordExistingSessionFileInput } from '@/lib/execution/remote-sandbox/session-file-provenance' -import { reportContentRefusal } from '@/lib/mothership/content-delivery/telemetry' +import { createContentRefusals } from '@/lib/mothership/content-delivery/telemetry' import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection' /** The workbench's history could not be widened, so nothing may enter the machine. */ @@ -55,11 +55,12 @@ export function createSessionContentGate(scope: { async admit(sources) { const verdicts: boolean[] = [] const admitted: DurableSecretProvenance[] = [] + const refusals = createContentRefusals(scope.workspaceId) let unrecorded = 0 for (const source of sources) { const { provenance } = source if (provenance.status === 'unknown') { - verdicts.push(reportContentRefusal(source, 'unknown', scope.workspaceId)) + verdicts.push(refusals.refuse(source, 'unknown')) continue } if ( @@ -67,7 +68,7 @@ export function createSessionContentGate(scope: { provenance.entries.length > 0 && source.view === 'excerpt' ) { - verdicts.push(reportContentRefusal(source, 'opaque', scope.workspaceId)) + verdicts.push(refusals.refuse(source, 'opaque')) continue } if (provenance.status === 'unrecorded') { @@ -78,6 +79,7 @@ export function createSessionContentGate(scope: { } verdicts.push(true) } + refusals.report() if (admitted.length > 0) { try { await recordExistingSessionFileInput( diff --git a/apps/sim/lib/mothership/content-delivery/telemetry.ts b/apps/sim/lib/mothership/content-delivery/telemetry.ts index 658aee2a55e..29d80b51cb3 100644 --- a/apps/sim/lib/mothership/content-delivery/telemetry.ts +++ b/apps/sim/lib/mothership/content-delivery/telemetry.ts @@ -1,8 +1,9 @@ import type { ContentSource, ContentSourceKind } from '@/lib/core/application' import { type DurableSecretProvenanceRefusalCause, + type DurableSecretProvenanceRefusalReport, type DurableSecretProvenanceSurface, - reportDurableSecretProvenanceRefusal, + reportDurableSecretProvenanceRefusals, } from '@/lib/execution/durable-secret-provenance-telemetry' export type ContentRefusalReason = 'unknown' | 'opaque' | 'registry' | 'import' @@ -55,17 +56,56 @@ const CAUSES: Record< }, } -/** Names the refused item and why, never its content. */ -export function reportContentRefusal( - source: ContentSource, - reason: ContentRefusalReason, - workspaceId: string | undefined -): false { - reportDurableSecretProvenanceRefusal({ - surface: CONTENT_SURFACE[source.kind], - cause: CAUSES[source.kind][reason], - workspaceId, - resourceId: source.id, +/** + * Collects the items one admission refuses and reports them once, naming each item and why, never + * its content. A page of legacy rows is one line, not one per row. + */ +export interface ContentRefusals { + /** Records the refusal of `source`; returns the verdict, `false`. */ + refuse(source: ContentSource, reason: ContentRefusalReason): false + /** Reports what was recorded, as one line. */ + report(): void +} + +export function createContentRefusals(workspaceId: string | undefined): ContentRefusals { + const reports: DurableSecretProvenanceRefusalReport[] = [] + return { + refuse(source: ContentSource, reason: ContentRefusalReason): false { + reports.push({ + surface: CONTENT_SURFACE[source.kind], + cause: CAUSES[source.kind][reason], + workspaceId, + resourceId: source.id, + }) + return false + }, + report(): void { + reportDurableSecretProvenanceRefusals(reports) + }, + } +} + +const WITHHELD_LABELS: Record = { + 'workspace-file': ['file', 'files'], + 'execution-file': ['run file', 'run files'], + 'knowledge-document': ['knowledge document', 'knowledge documents'], + 'knowledge-chunk': ['knowledge chunk', 'knowledge chunks'], + 'table-row': ['table row', 'table rows'], + 'tool-result': ['tool result', 'tool results'], +} + +/** + * Tells Chat's model what a result left out: a refused item is dropped from the page it was read + * in, which otherwise reads exactly like an item that does not exist. + */ +export function describeWithheldContent( + withheld: ReadonlyMap +): string | undefined { + const lines = [...withheld].map(([kind, count]) => { + const [singular, plural] = WITHHELD_LABELS[kind] + return count === 1 + ? `Note: 1 ${singular} was withheld from this result because its secret provenance is unavailable. It exists; reading it again will not return it.` + : `Note: ${count} ${plural} were withheld from this result because their secret provenance is unavailable. They exist; reading them again will not return them.` }) - return false + return lines.length > 0 ? lines.join('\n') : undefined } diff --git a/apps/sim/lib/mothership/generated/billing.ts b/apps/sim/lib/mothership/generated/billing.ts index abdec1159ad..cf01b6b4b98 100644 --- a/apps/sim/lib/mothership/generated/billing.ts +++ b/apps/sim/lib/mothership/generated/billing.ts @@ -90,6 +90,8 @@ export const UsageLimitRefusal = z.object({ usageUpgrade: UsageUpgrade.optional().catch(undefined), }); export const BillingDuplicateCode = "DUPLICATE_BILLING_EVENT"; +/** A 503: another attempt of the same request is still being applied, so this one's outcome is unknown. */ +export const BillingInFlightCode = "BILLING_REQUEST_IN_FLIGHT"; /** Sim authors receipts at the paid execution boundary; tool output carries no billing authority. */ export const ServiceUsageReceipt = z.strictObject({ diff --git a/apps/sim/lib/table/rows/secret-provenance.ts b/apps/sim/lib/table/rows/secret-provenance.ts index 1d49c72e252..82bf9c370b6 100644 --- a/apps/sim/lib/table/rows/secret-provenance.ts +++ b/apps/sim/lib/table/rows/secret-provenance.ts @@ -988,8 +988,9 @@ function envelopeFromClassifications( rowCount: number, scope: ResolvedSecretTraceScopeV1 ): ResolvedSecretTraceProvenanceV1 { + /** Legacy or stale rows read as unknown by policy; a writer that failed to record is logged where it wrote. */ const incomplete = (cause: string): ResolvedSecretTraceProvenanceV1 => { - logger.error('Table row read could not establish secret provenance', { + logger.warn('Table row read could not establish secret provenance', { surface: 'table-row', cause, rowCount, diff --git a/packages/testing/src/mocks/billing-usage-log.mock.ts b/packages/testing/src/mocks/billing-usage-log.mock.ts index 9e1133828fa..ff7def0f720 100644 --- a/packages/testing/src/mocks/billing-usage-log.mock.ts +++ b/packages/testing/src/mocks/billing-usage-log.mock.ts @@ -39,6 +39,14 @@ export class MockCumulativeUsagePeriodClosedError extends Error { } } +/** Stand-in for `CumulativeUsageInFlightError` with the real `name`, `eventKey`, and message. */ +export class MockCumulativeUsageInFlightError extends Error { + constructor(readonly eventKey: string) { + super(`Cumulative usage event "${eventKey}" is already being recorded`) + this.name = 'CumulativeUsageInFlightError' + } +} + /** * Stand-in for `UnknownUsageCursorError` with the real `name`, message, and `statusCode` 400. * It is NOT a subclass of the real `HttpError`, and its `cause` is a plain `Error` carrying @@ -107,7 +115,8 @@ export const billingUsageLogMockFns = { /** * Static mock module for `@/lib/billing/core/usage-log`. Constants carry the real values; * `CumulativeUsageContextMismatchError` is {@link MockCumulativeUsageContextMismatchError}, - * `CumulativeUsagePeriodClosedError` is {@link MockCumulativeUsagePeriodClosedError}, and + * `CumulativeUsagePeriodClosedError` is {@link MockCumulativeUsagePeriodClosedError}, + * `CumulativeUsageInFlightError` is {@link MockCumulativeUsageInFlightError}, and * `UnknownUsageCursorError` is {@link MockUnknownUsageCursorError}. * * @example @@ -122,6 +131,7 @@ export const billingUsageLogMock = { UNKNOWN_CURSOR_MESSAGE, CumulativeUsageContextMismatchError: MockCumulativeUsageContextMismatchError, CumulativeUsagePeriodClosedError: MockCumulativeUsagePeriodClosedError, + CumulativeUsageInFlightError: MockCumulativeUsageInFlightError, UnknownUsageCursorError: MockUnknownUsageCursorError, isUnbilledUsageCategory: billingUsageLogMockFns.mockIsUnbilledUsageCategory, stableEventKey: billingUsageLogMockFns.mockStableEventKey, diff --git a/packages/testing/src/mocks/index.ts b/packages/testing/src/mocks/index.ts index 212acf24b63..f32c784e589 100644 --- a/packages/testing/src/mocks/index.ts +++ b/packages/testing/src/mocks/index.ts @@ -146,6 +146,7 @@ export { billingUsageLogMock, billingUsageLogMockFns, MockCumulativeUsageContextMismatchError, + MockCumulativeUsageInFlightError, MockCumulativeUsagePeriodClosedError, MockUnknownUsageCursorError, } from './billing-usage-log.mock' From b47b4079065af4112de52157d526877843203b4b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 15 Oct 2026 16:31:23 -0700 Subject: [PATCH 2/7] test(billing): model the ledger try-lock in the unit tx mock; pass the route context The usage-log unit mock answered every statement with undefined, which the try-lock cannot read; it now answers whether the lock was taken, with a test that a held lock refuses before any read or write. The new route test passes the route context like the typed handler expects, and the refusal reason type stays module-private. --- .../app/api/billing/update-cost/route.test.ts | 4 +++- apps/sim/lib/billing/core/usage-log.test.ts | 23 +++++++++++++++++-- .../mothership/content-delivery/telemetry.ts | 2 +- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/apps/sim/app/api/billing/update-cost/route.test.ts b/apps/sim/app/api/billing/update-cost/route.test.ts index 826e1bfc737..a0098ea6995 100644 --- a/apps/sim/app/api/billing/update-cost/route.test.ts +++ b/apps/sim/app/api/billing/update-cost/route.test.ts @@ -1,5 +1,6 @@ import { readFileSync } from 'node:fs' import { createMockRequest, resetEnvFlagsMock, setEnvFlags } from '@sim/testing' +import { createRouteContext } from '@sim/testing/helpers/http' import { billingAttributionMock, billingAttributionMockFns, @@ -772,7 +773,8 @@ describe('POST /api/billing/update-cost — workspaceId attribution', () => { 'x-sim-billing-request-id': billingRequestId, 'x-sim-billing-account-decision': 'serialized-account-decision', } - ) + ), + createRouteContext({}) ) expect(res.status).toBe(503) diff --git a/apps/sim/lib/billing/core/usage-log.test.ts b/apps/sim/lib/billing/core/usage-log.test.ts index 9a1d2d73523..a18b4850ef1 100644 --- a/apps/sim/lib/billing/core/usage-log.test.ts +++ b/apps/sim/lib/billing/core/usage-log.test.ts @@ -31,6 +31,7 @@ import { USAGE_LEDGER_STATEMENT_TIMEOUT_MS } from '@/lib/billing/constants' import { CUMULATIVE_COST_EPSILON, CumulativeUsageContextMismatchError, + CumulativeUsageInFlightError, getBillingPeriodUsageCost, getBillingPeriodUsageCostByUser, getBillingPeriodUsageCostWithSourceSubset, @@ -245,7 +246,8 @@ describe('recordCumulativeUsage', () => { mockIsOrgScopedSubscription.mockReturnValue(true) }) - const setupTx = (existingRow: Partial | null) => { + /** `locked`: another attempt of the request holds its ledger lock. */ + const setupTx = (existingRow: Partial | null, locked = false) => { const resolvedExistingRow = existingRow ? { ...defaultExistingRow, ...existingRow } : null const limit = vi.fn().mockResolvedValue(resolvedExistingRow ? [resolvedExistingRow] : []) const where = vi.fn().mockReturnValue({ limit }) @@ -255,7 +257,8 @@ describe('recordCumulativeUsage', () => { const updateSet = vi.fn().mockReturnValue({ where: updateWhere }) mockUpdate.mockReturnValue({ set: updateSet }) const tx = { - execute: vi.fn().mockResolvedValue(undefined), + // The configure statement's result is unused; the try-lock reads `acquired`. + execute: vi.fn().mockResolvedValue([{ acquired: !locked }]), select, update: mockUpdate, insert: mockInsert, // recordUsage(tx) reuses the shared insert chain @@ -290,6 +293,22 @@ describe('recordCumulativeUsage', () => { }) }) + it('refuses at once, writing nothing, while another attempt of the request holds its lock', async () => { + const { select } = setupTx(null, true) + await expect( + recordCumulativeUsage({ + userId: 'user-1', + source: 'workspace-chat', + model: 'claude-opus-4.8', + cost: 0.4662453, + eventKey: 'update-cost:msg-1-billing', + }) + ).rejects.toBeInstanceOf(CumulativeUsageInFlightError) + expect(select).not.toHaveBeenCalled() + expect(mockInsert).not.toHaveBeenCalled() + expect(mockUpdate).not.toHaveBeenCalled() + }) + it('tops up to the higher cumulative and bills only the delta', async () => { const { updateSet } = setupTx({ id: 'row-1', cost: '0.3474447' }) const result = await recordCumulativeUsage({ diff --git a/apps/sim/lib/mothership/content-delivery/telemetry.ts b/apps/sim/lib/mothership/content-delivery/telemetry.ts index 29d80b51cb3..914ec94b2d7 100644 --- a/apps/sim/lib/mothership/content-delivery/telemetry.ts +++ b/apps/sim/lib/mothership/content-delivery/telemetry.ts @@ -6,7 +6,7 @@ import { reportDurableSecretProvenanceRefusals, } from '@/lib/execution/durable-secret-provenance-telemetry' -export type ContentRefusalReason = 'unknown' | 'opaque' | 'registry' | 'import' +type ContentRefusalReason = 'unknown' | 'opaque' | 'registry' | 'import' export const CONTENT_SURFACE: Record = { 'workspace-file': 'workspace-file', From 31e7cfe79d3d71edac22ab80e0764916b9fc6989 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 15 Oct 2026 16:37:15 -0700 Subject: [PATCH 3/7] fix(mothership): word the withheld note for every refusal, and describe the rows that read as unknown A gate refuses either the item (unknown provenance) or only the requested view (an excerpt or raw bytes redaction cannot cover), so the note no longer says another read cannot return it. Untracked legacy rows read as exact-empty; only tracked rows with a stale, missing or malformed sidecar read as unknown. --- apps/sim/lib/mothership/agent-cli/file-provenance.test.ts | 2 +- apps/sim/lib/mothership/content-delivery/telemetry.ts | 8 +++++--- apps/sim/lib/table/rows/secret-provenance.ts | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts b/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts index 9c087594faa..5f8d890045d 100644 --- a/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts +++ b/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts @@ -572,7 +572,7 @@ describe('file provenance at the actual CLI and model-result boundary', () => { /** The model is told the file it cannot see exists, so it does not read the gap as absence. */ expect(result.output).toMatchObject({ stderr: expect.stringContaining( - 'Note: 1 file was withheld from this result because its secret provenance is unavailable.' + 'Note: 1 file was withheld from this result because it could not be shown safely' ), }) }) diff --git a/apps/sim/lib/mothership/content-delivery/telemetry.ts b/apps/sim/lib/mothership/content-delivery/telemetry.ts index 914ec94b2d7..e7662d30040 100644 --- a/apps/sim/lib/mothership/content-delivery/telemetry.ts +++ b/apps/sim/lib/mothership/content-delivery/telemetry.ts @@ -96,7 +96,9 @@ const WITHHELD_LABELS: Record = { /** * Tells Chat's model what a result left out: a refused item is dropped from the page it was read - * in, which otherwise reads exactly like an item that does not exist. + * in, which otherwise reads exactly like an item that does not exist. A refusal can be of the + * item (its provenance is unknown) or only of the view (an excerpt or raw bytes redaction cannot + * cover), so the note says it exists and claims nothing about other reads. */ export function describeWithheldContent( withheld: ReadonlyMap @@ -104,8 +106,8 @@ export function describeWithheldContent( const lines = [...withheld].map(([kind, count]) => { const [singular, plural] = WITHHELD_LABELS[kind] return count === 1 - ? `Note: 1 ${singular} was withheld from this result because its secret provenance is unavailable. It exists; reading it again will not return it.` - : `Note: ${count} ${plural} were withheld from this result because their secret provenance is unavailable. They exist; reading them again will not return them.` + ? `Note: 1 ${singular} was withheld from this result because it could not be shown safely (its secret provenance is unknown, or this view of it cannot be redacted). It exists; do not treat it as missing.` + : `Note: ${count} ${plural} were withheld from this result because they could not be shown safely (their secret provenance is unknown, or this view of them cannot be redacted). They exist; do not treat them as missing.` }) return lines.length > 0 ? lines.join('\n') : undefined } diff --git a/apps/sim/lib/table/rows/secret-provenance.ts b/apps/sim/lib/table/rows/secret-provenance.ts index 82bf9c370b6..4e5d1d0cac6 100644 --- a/apps/sim/lib/table/rows/secret-provenance.ts +++ b/apps/sim/lib/table/rows/secret-provenance.ts @@ -988,7 +988,7 @@ function envelopeFromClassifications( rowCount: number, scope: ResolvedSecretTraceScopeV1 ): ResolvedSecretTraceProvenanceV1 { - /** Legacy or stale rows read as unknown by policy; a writer that failed to record is logged where it wrote. */ + /** A tracked row whose sidecar is stale, missing or malformed reads as unknown by policy; writer faults log where they write. */ const incomplete = (cause: string): ResolvedSecretTraceProvenanceV1 => { logger.warn('Table row read could not establish secret provenance', { surface: 'table-row', From 5b409991d7618451ce56756cc1607d28f224e33a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 15 Oct 2026 17:02:11 -0700 Subject: [PATCH 4/7] fix(provenance): keep refused ids paired with their cause, name sandbox files by hash, and tell Chat why each item was withheld Refusal reports are grouped by surface, cause and owning record (document, table or chat), each line with top-level surface, cause and refusedCount and the refused ids (up to 200, flagged when truncated), so per-cause queries and id-to-cause pairing survive aggregation. A sandbox file is attributed by a chat-scoped hash of its path rather than the path, which carries the user's file name. A streamed sandbox image declares its view opaque, so its refusal reads as opaque content rather than a missing registry; the use case now requires every caller to declare the view. Gate verdicts carry the refusal reason, and Chat's CLI note says per reason what a read can do next: unknown provenance will not return on a retry, recorded secrets can be read as text for a redacted copy, and provenance that could not be verified may be readable later. Chunk write refusals name their document as the parent. --- .../core/application/content-delivery.test.ts | 25 ++++++- .../lib/core/application/content-delivery.ts | 43 +++++++++--- apps/sim/lib/core/application/index.ts | 3 + ...urable-secret-provenance-telemetry.test.ts | 57 +++++++++++---- .../durable-secret-provenance-telemetry.ts | 55 +++++++++------ .../application/admit-knowledge-content.ts | 1 + apps/sim/lib/knowledge/application/chunks.ts | 4 +- .../agent-cli/engines/scratch-file-read.ts | 1 + .../agent-cli/file-provenance.test.ts | 2 +- .../application/inline-images-scratch.test.ts | 59 +++++++++++++++- .../chat/application/inline-images.ts | 2 +- .../application/read-sandbox-file.test.ts | 7 +- .../chat/application/read-sandbox-file.ts | 16 ++++- .../content-delivery/registry-gate.test.ts | 20 +++--- .../content-delivery/registry-gate.ts | 6 +- .../content-delivery/session-gate.test.ts | 6 +- .../content-delivery/session-gate.ts | 4 +- .../content-delivery/telemetry.test.ts | 40 +++++++++++ .../mothership/content-delivery/telemetry.ts | 70 +++++++++++++------ apps/sim/lib/table/application/rows.test.ts | 8 ++- apps/sim/lib/table/application/rows.ts | 41 ++++++++--- 21 files changed, 361 insertions(+), 109 deletions(-) create mode 100644 apps/sim/lib/mothership/content-delivery/telemetry.test.ts diff --git a/apps/sim/lib/core/application/content-delivery.test.ts b/apps/sim/lib/core/application/content-delivery.test.ts index a849c87cd94..037f25c7057 100644 --- a/apps/sim/lib/core/application/content-delivery.test.ts +++ b/apps/sim/lib/core/application/content-delivery.test.ts @@ -523,9 +523,28 @@ describe('countWithheldContent', () => { }) ) expect(result).toBe(1) - expect([...withheld]).toEqual([ - ['table-row', 2], - ['workspace-file', 1], + expect([...withheld].map(([kind, reasons]) => [kind, [...reasons]])).toEqual([ + ['table-row', [['unknown', 2]]], + ['workspace-file', [['unknown', 1]]], + ]) + }) + + it('keeps the reason a gate gave for each refusal', async () => { + const gate: ContentDeliveryGate = { + async admit(sources) { + return sources.map((item) => + item.id === 'raw' ? 'opaque' : item.id === 'later' ? 'registry' : true + ) + }, + } + const { withheld } = await observeContentDelivery(gate, () => + countWithheldContent(() => + admitContent(CHAT, [source('raw'), source('later'), source('clean')]) + ) + ) + expect([...(withheld.get('workspace-file') ?? [])]).toEqual([ + ['opaque', 1], + ['registry', 1], ]) }) diff --git a/apps/sim/lib/core/application/content-delivery.ts b/apps/sim/lib/core/application/content-delivery.ts index 1ea2b03a563..23741c8dea1 100644 --- a/apps/sim/lib/core/application/content-delivery.ts +++ b/apps/sim/lib/core/application/content-delivery.ts @@ -31,18 +31,36 @@ export type ContentView = 'complete' | 'opaque' | 'excerpt' | { derived: unknown /** One stored item or tool result whose content a use case is about to return. */ export interface ContentSource { kind: ContentSourceKind - /** Canonical id, for attribution and per-item exclusion only. */ + /** Canonical id, for attribution and per-item exclusion only. Never a name or path. */ id: string + /** The record that owns the item (its document, table or chat), for attribution only. */ + parentId?: string provenance: ContentProvenance view: ContentView } +/** + * Why a gate refused an item. `unknown`: its secret provenance is unknown, so no read returns it. + * `opaque`: it holds recorded secrets in a view no redaction reaches (raw bytes, an excerpt), while + * its full text can be redacted. `registry`, `import`: its provenance could not be checked now. + */ +export type ContentRefusalReason = 'unknown' | 'opaque' | 'registry' | 'import' + +/** `true` admits; a reason refuses. A bare `false` refuses for an unstated reason, read as `unknown`. */ +export type ContentVerdict = boolean | ContentRefusalReason + +/** What a gate refused inside a {@link countWithheldContent} call, by kind and then reason. */ +export type WithheldContent = ReadonlyMap< + ContentSourceKind, + ReadonlyMap +> + /** * Decides, per item, whether content may leave through the surface that installed it. A gate never * throws for a refusal: refusing one item must not cost the call the items it admits. */ export interface ContentDeliveryGate { - admit(sources: readonly ContentSource[]): Promise + admit(sources: readonly ContentSource[]): Promise /** * Redacts text before cutting, or returns `undefined` when it cannot be shown. The current * operation's bounded evidence lets a surface create a redactor only when a cut needs one. @@ -73,8 +91,8 @@ interface ContentLedger { accounted: boolean /** Bounded aggregate of admitted provenance, for a surface that persists the delivered bytes. */ admitted: ContentProvenance[] - /** Only on a {@link countWithheldContent} ledger: the items refused inside it, by kind. */ - withheld?: Map + /** Only on a {@link countWithheldContent} ledger: the items refused inside it. */ + withheld?: Map> /** A committed write's returned item was refused; the operation answers with a refusal. */ withheldAfterWrite?: ContentSourceKind /** The gate itself failed while this ledger was open, whatever the route then answered. */ @@ -207,7 +225,7 @@ export function mustClassifyContent(principal: Principal): boolean { export class ContentAdmission { private readonly verdicts: ReadonlyMap - constructor(sources: readonly ContentSource[], verdicts: readonly boolean[]) { + constructor(sources: readonly ContentSource[], verdicts: readonly ContentVerdict[]) { this.verdicts = new Map(sources.map((source, index) => [source, verdicts[index] === true])) } @@ -293,7 +311,7 @@ export async function admitContent( const gate = gates.getStore() if (!gate) throw new ContentDeliveryUnobservedError() const ledger = ledgers.getStore() - let verdicts: readonly boolean[] + let verdicts: readonly ContentVerdict[] try { verdicts = await gate.admit(sources) } catch (error) { @@ -309,13 +327,18 @@ export async function admitContent( const { withheld } = current if (!withheld) continue sources.forEach(({ kind }, index) => { - if (!verdicts[index]) withheld.set(kind, (withheld.get(kind) ?? 0) + 1) + const verdict = verdicts[index] + if (verdict === true) return + const reason = typeof verdict === 'string' ? verdict : 'unknown' + const reasons = withheld.get(kind) ?? new Map() + reasons.set(reason, (reasons.get(reason) ?? 0) + 1) + withheld.set(kind, reasons) }) } const batch = mergeContentProvenance( (function* () { for (let index = 0; index < sources.length; index++) { - if (verdicts[index]) yield sources[index].provenance + if (verdicts[index] === true) yield sources[index].provenance } })() ) @@ -475,8 +498,8 @@ export async function trackContentDelivery( */ export async function countWithheldContent( execute: () => Promise -): Promise<{ result: T; withheld: ReadonlyMap }> { - const withheld = new Map() +): Promise<{ result: T; withheld: WithheldContent }> { + const withheld = new Map>() const ledger: ContentLedger = { parent: ledgers.getStore(), sourcedOperationAdmitted: false, diff --git a/apps/sim/lib/core/application/index.ts b/apps/sim/lib/core/application/index.ts index 572006c2c77..47b0b126ef3 100644 --- a/apps/sim/lib/core/application/index.ts +++ b/apps/sim/lib/core/application/index.ts @@ -70,9 +70,11 @@ export { ContentDeliveryUnobservedError, ContentGateFailure, type ContentProvenance, + type ContentRefusalReason, ContentRefusedError, type ContentSource, type ContentSourceKind, + type ContentVerdict, type ContentView, countWithheldContent, isCopilotPrincipal, @@ -84,4 +86,5 @@ export { requireAdmittedContent, runDeclaredContentOperation, trackContentDelivery, + type WithheldContent, } from './content-delivery' diff --git a/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts b/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts index 526f1b0a7a4..473f3941446 100644 --- a/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts +++ b/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts @@ -53,30 +53,59 @@ describe('durable secret provenance telemetry', () => { } ) }) - it('reports a batch of refusals as one line with counts and a bounded sample', () => { + it('reports a batch once per surface, cause and owner, keeping every id paired with its cause', () => { logger.warn.mockClear() - const reports = Array.from({ length: 8 }, (_, index) => ({ - surface: 'table-row' as const, + const reports = Array.from({ length: 163 }, (_, index) => ({ + surface: 'knowledge' as const, cause: - index < 6 - ? ('table-row-provenance-unavailable' as const) - : ('table-row-opaque-secret-content' as const), + index < 160 + ? ('knowledge-chunk-source-unavailable' as const) + : ('knowledge-opaque-secret-content' as const), workspaceId: 'workspace-1', - resourceId: `row-${index}`, + parentId: 'document-1', + resourceId: `chunk-${index}`, })) reportDurableSecretProvenanceRefusals(reports) - expect(logger.warn).toHaveBeenCalledExactlyOnceWith( + expect(logger.warn).toHaveBeenCalledTimes(2) + expect(logger.warn).toHaveBeenNthCalledWith( + 1, 'Refusing unavailable durable secret provenance', { - refusedCount: 8, - causes: { - 'table-row:table-row-provenance-unavailable': 6, - 'table-row:table-row-opaque-secret-content': 2, - }, + surface: 'knowledge', + cause: 'knowledge-chunk-source-unavailable', + refusedCount: 160, workspaceId: 'workspace-1', - sampleResourceIds: ['row-0', 'row-1', 'row-2', 'row-3', 'row-4'], + parentId: 'document-1', + resourceIds: reports.slice(0, 160).map((report) => report.resourceId), + } + ) + expect(logger.warn).toHaveBeenNthCalledWith( + 2, + 'Refusing unavailable durable secret provenance', + { + surface: 'knowledge', + cause: 'knowledge-opaque-secret-content', + refusedCount: 3, + workspaceId: 'workspace-1', + parentId: 'document-1', + resourceIds: ['chunk-160', 'chunk-161', 'chunk-162'], } ) expect(logger.error).not.toHaveBeenCalled() }) + + it('caps the ids of one very large group and says so', () => { + logger.warn.mockClear() + reportDurableSecretProvenanceRefusals( + Array.from({ length: 250 }, (_, index) => ({ + surface: 'table-row' as const, + cause: 'table-row-provenance-unavailable' as const, + parentId: 'table-1', + resourceId: `row-${index}`, + })) + ) + const [[, fields]] = logger.warn.mock.calls + expect(fields).toMatchObject({ refusedCount: 250, resourceIdsTruncated: true }) + expect(fields.resourceIds).toHaveLength(200) + }) }) diff --git a/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts b/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts index 4c82f29e6f2..5f6dfa5943b 100644 --- a/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts +++ b/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts @@ -50,7 +50,10 @@ export interface DurableSecretProvenanceRefusalReport { surface: DurableSecretProvenanceSurface cause: DurableSecretProvenanceRefusalCause workspaceId?: string + /** An opaque id, never a name or path. */ resourceId?: string + /** The record that owns the resource (its document, table or chat). */ + parentId?: string } /** @@ -81,38 +84,46 @@ export function reportDurableSecretProvenanceRefusal( surface: report.surface, cause: report.cause, ...(report.workspaceId ? { workspaceId: report.workspaceId } : {}), + ...(report.parentId ? { parentId: report.parentId } : {}), ...(report.resourceId ? { resourceId: report.resourceId } : {}), }) } -/** Resource ids a batched refusal names, enough to find the items without listing a whole page. */ -const REFUSAL_SAMPLE_SIZE = 5 +/** Ids one grouped refusal lists; the ids are opaque, and a page of them stays one line. */ +const REFUSAL_RESOURCE_ID_LIMIT = 200 -/** Reports every refusal of one read as a single line: counts per surface and cause, and a sample. */ +/** + * Reports the refusals of one read with one line per surface, cause and owning record, each + * carrying the same top-level `surface` and `cause` as a single refusal, its `refusedCount`, and + * the refused ids, so each id stays paired with its cause. + */ export function reportDurableSecretProvenanceRefusals( reports: readonly DurableSecretProvenanceRefusalReport[] ): void { - const [first] = reports - if (!first) return - if (reports.length === 1) { - reportDurableSecretProvenanceRefusal(first) - return - } - const causes: Record = {} + const groups = new Map() for (const report of reports) { - const key = `${report.surface}:${report.cause}` - causes[key] = (causes[key] ?? 0) + 1 + const key = JSON.stringify([report.surface, report.cause, report.workspaceId, report.parentId]) + const group = groups.get(key) + if (group) group.push(report) + else groups.set(key, [report]) + } + for (const group of groups.values()) { + const [first] = group + if (group.length === 1) { + reportDurableSecretProvenanceRefusal(first) + continue + } + const resourceIds = group.flatMap((report) => (report.resourceId ? [report.resourceId] : [])) + persistenceLogger.warn('Refusing unavailable durable secret provenance', { + surface: first.surface, + cause: first.cause, + refusedCount: group.length, + ...(first.workspaceId ? { workspaceId: first.workspaceId } : {}), + ...(first.parentId ? { parentId: first.parentId } : {}), + resourceIds: resourceIds.slice(0, REFUSAL_RESOURCE_ID_LIMIT), + ...(resourceIds.length > REFUSAL_RESOURCE_ID_LIMIT ? { resourceIdsTruncated: true } : {}), + }) } - const workspaceIds = new Set(reports.map((report) => report.workspaceId)) - const sampleResourceIds = reports - .flatMap((report) => (report.resourceId ? [report.resourceId] : [])) - .slice(0, REFUSAL_SAMPLE_SIZE) - persistenceLogger.warn('Refusing unavailable durable secret provenance', { - refusedCount: reports.length, - causes, - ...(workspaceIds.size === 1 && first.workspaceId ? { workspaceId: first.workspaceId } : {}), - ...(sampleResourceIds.length > 0 ? { sampleResourceIds } : {}), - }) } export interface DurableSecretProvenanceUnrecordedReport { diff --git a/apps/sim/lib/knowledge/application/admit-knowledge-content.ts b/apps/sim/lib/knowledge/application/admit-knowledge-content.ts index 736322e5cdf..acba3188330 100644 --- a/apps/sim/lib/knowledge/application/admit-knowledge-content.ts +++ b/apps/sim/lib/knowledge/application/admit-knowledge-content.ts @@ -81,6 +81,7 @@ export async function admitKnowledgeChunks( return items.map((chunk) => ({ kind: 'knowledge-chunk' as const, id: chunk.id, + parentId: chunk.documentId, provenance: provenance.get(chunk.id) ?? { status: 'unknown' as const }, view: chunk.value === undefined ? ('complete' as const) : { derived: chunk.value }, })) diff --git a/apps/sim/lib/knowledge/application/chunks.ts b/apps/sim/lib/knowledge/application/chunks.ts index 719f87d56ed..f55469b2787 100644 --- a/apps/sim/lib/knowledge/application/chunks.ts +++ b/apps/sim/lib/knowledge/application/chunks.ts @@ -212,7 +212,7 @@ export const createKnowledgeChunk = defineAuthorizedKnowledgeUseCase({ surface: 'knowledge', cause: 'knowledge-chunk-source-unavailable', workspaceId: context.workspaceId, - resourceId: context.documentId, + parentId: context.documentId, }) throw new OrchestrationError('validation', 'Knowledge chunk secret provenance is unavailable') } @@ -288,7 +288,7 @@ export const updateKnowledgeChunk = defineAuthorizedKnowledgeUseCase({ surface: 'knowledge', cause: 'knowledge-chunk-source-unavailable', workspaceId: context.workspaceId, - resourceId: context.documentId, + parentId: context.documentId, }) throw new OrchestrationError('validation', 'Knowledge chunk secret provenance is unavailable') } diff --git a/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts b/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts index accf2cd0553..e4590cadeeb 100644 --- a/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts +++ b/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts @@ -20,6 +20,7 @@ export async function readScratchFile( ? { organizationId: runtime.chatOrganizationId } : { workspaceId: runtime.workspaceId }), path: reference, + view: 'complete', maxBytes: options.maxBytes, signal: runtime.signal, }, diff --git a/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts b/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts index 5f8d890045d..96a707dbcfa 100644 --- a/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts +++ b/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts @@ -572,7 +572,7 @@ describe('file provenance at the actual CLI and model-result boundary', () => { /** The model is told the file it cannot see exists, so it does not read the gap as absence. */ expect(result.output).toMatchObject({ stderr: expect.stringContaining( - 'Note: 1 file was withheld from this result because it could not be shown safely' + 'Note: 1 file was withheld from this result: its secret provenance is unknown, so reading it again will not return it.' ), }) }) diff --git a/apps/sim/lib/mothership/chat/application/inline-images-scratch.test.ts b/apps/sim/lib/mothership/chat/application/inline-images-scratch.test.ts index 11c5afbc976..967e60eda14 100644 --- a/apps/sim/lib/mothership/chat/application/inline-images-scratch.test.ts +++ b/apps/sim/lib/mothership/chat/application/inline-images-scratch.test.ts @@ -1,6 +1,7 @@ import { copilotChats } from '@sim/db/schema' import { queueTableRows, resetDbChainMock } from '@sim/testing' import { authBanMock } from '@sim/testing/mocks/auth-ban.mock' +import { getMockLogger } from '@sim/testing/mocks/logger.mock' import { remoteSandboxMock } from '@sim/testing/mocks/remote-sandbox.mock' import { storageServiceMock, storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock' import { workspaceAuthzMock, workspaceAuthzMockFns } from '@sim/testing/mocks/workspace-authz.mock' @@ -8,7 +9,11 @@ import { workspaceContextMock } from '@sim/testing/mocks/workspace-context.mock' import sharp from 'sharp' import { beforeEach, expect, it, vi } from 'vitest' -const hoisted = vi.hoisted(() => ({ snapshot: vi.fn(), dispose: vi.fn() })) +const hoisted = vi.hoisted(() => ({ + snapshot: vi.fn(), + dispose: vi.fn(), + provenance: vi.fn(() => ({ status: 'exact', entries: [] as unknown[] })), +})) vi.mock('@sim/platform-authz/workspace', () => workspaceAuthzMock) vi.mock('@/lib/auth/ban', () => authBanMock) vi.mock('@/lib/workspaces/application/context', () => workspaceContextMock) @@ -25,7 +30,7 @@ vi.mock('@/lib/execution/remote-sandbox/session-file-snapshot', () => ({ vi.mock('@/lib/mothership/agent-cli/workbench-file-provenance', () => ({ createWorkbenchFileProvenance: () => ({ observeUpload: (_machine: unknown, stream: unknown) => stream, - uploadProvenance: () => ({ status: 'exact', entries: [] }), + uploadProvenance: () => hoisted.provenance(), }), })) vi.mock('@/lib/execution/remote-sandbox', () => remoteSandboxMock) @@ -83,3 +88,53 @@ it('runs the real nested scratch authorization, limit validation, snapshot read expect.objectContaining({ createOnly: true, persistMetadata: false, contentType: 'image/webp' }) ) }) + +it('refuses an image holding recorded secrets as opaque, naming it without its path', async () => { + const owner = { + userId: 'owner', + workspaceId: 'workspace', + organizationId: null, + type: 'mothership', + } + queueTableRows(copilotChats, [owner]) + queueTableRows(copilotChats, [owner]) + hoisted.provenance.mockReturnValueOnce({ + status: 'exact', + entries: [ + { + encryptedValue: 'ciphertext', + sourceUserId: 'owner', + sourceWorkspaceId: 'workspace', + name: 'FIXTURE_TOKEN', + }, + ], + }) + const buffer = Buffer.from('not shown') + mocks.snapshot.mockResolvedValue({ + size: buffer.length, + dispose: mocks.dispose, + stream: async () => + new ReadableStream({ + start(controller) { + controller.enqueue(buffer) + controller.close() + }, + }), + }) + const refusals = getMockLogger('DurableSecretProvenancePersistence').warn + refusals.mockClear() + await expect( + materializeStreamImage( + { userId: 'owner', workspaceId: 'workspace', chatId: 'chat' }, + { requestId: 'request-secret', reference: '/tmp/quarterly%20payroll.png' } + ) + ).rejects.toThrow() + expect(refusals).toHaveBeenCalledTimes(1) + const [, fields] = refusals.mock.calls[0] + expect(fields).toMatchObject({ + surface: 'workspace-file', + cause: 'workspace-file-opaque-secret-content', + parentId: 'chat', + }) + expect(JSON.stringify(fields)).not.toMatch(/payroll|\/tmp/) +}) diff --git a/apps/sim/lib/mothership/chat/application/inline-images.ts b/apps/sim/lib/mothership/chat/application/inline-images.ts index 2c1dd3f388d..d0aeac1af60 100644 --- a/apps/sim/lib/mothership/chat/application/inline-images.ts +++ b/apps/sim/lib/mothership/chat/application/inline-images.ts @@ -140,7 +140,7 @@ export const materializeInlineChatImage = defineAuthorizedChatUseCase({ source = ( await readChatSandboxFile.execute({ principal, - input: { ...scope, path: decodeURIComponent(reference), signal }, + input: { ...scope, path: decodeURIComponent(reference), view: 'opaque', signal }, request, }) ).buffer diff --git a/apps/sim/lib/mothership/chat/application/read-sandbox-file.test.ts b/apps/sim/lib/mothership/chat/application/read-sandbox-file.test.ts index edf57de8da5..41173657785 100644 --- a/apps/sim/lib/mothership/chat/application/read-sandbox-file.test.ts +++ b/apps/sim/lib/mothership/chat/application/read-sandbox-file.test.ts @@ -56,7 +56,12 @@ function readAsChat(scope: { principal?: Principal } = {}) { () => readChatSandboxFile.execute({ principal: scope.principal ?? chatPrincipal, input }) ) } -const input = { workspaceId: 'ws', chatId: 'chat', path: '/tmp/image.png' } +const input = { + workspaceId: 'ws', + chatId: 'chat', + path: '/tmp/image.png', + view: 'complete' as const, +} const machine = { providerId: 'e2b', sandboxId: 'physical' } beforeEach(() => { mocks.permission.mockResolvedValue('read') diff --git a/apps/sim/lib/mothership/chat/application/read-sandbox-file.ts b/apps/sim/lib/mothership/chat/application/read-sandbox-file.ts index f31b9a86c1b..3b020919ba6 100644 --- a/apps/sim/lib/mothership/chat/application/read-sandbox-file.ts +++ b/apps/sim/lib/mothership/chat/application/read-sandbox-file.ts @@ -1,5 +1,6 @@ import { posix } from 'node:path' import type { Principal } from '@sim/auth/principal' +import { sha256Hex } from '@sim/security/hash' import { defineWorkspaceOperation, requireAdmittedContent } from '@/lib/core/application' import { defineOrganizationOperation } from '@/lib/core/application/organization-operation' import { OrchestrationError } from '@/lib/core/orchestration/types' @@ -32,9 +33,19 @@ interface ReadSandboxFileInput { organizationId?: string path: string maxBytes?: number + /** How the bytes leave: as text redaction can cover, or `opaque` (an image) it cannot reach. */ + view: 'complete' | 'opaque' signal?: AbortSignal } +/** + * Names a sandbox file in provenance attribution without its path, which carries the user's file + * name: stable for one path in one chat, and meaningless outside it. + */ +function sandboxFileAttributionId(chatId: string, path: string): string { + return `sandbox:${sha256Hex(`${chatId}\0${path}`).slice(0, 32)}` +} + /** Reads a fixed snapshot from the caller's existing private chat machine, never server disk. */ export const readChatSandboxFile = defineAuthorizedChatUseCase({ operation: defineWorkspaceOperation({ @@ -114,9 +125,10 @@ export const readChatSandboxFile = defineAuthorizedChatUseCase({ const receipt = provenance.uploadProvenance() await requireAdmittedContent(principal, { kind: 'workspace-file', - id: path, + id: sandboxFileAttributionId(context.chatId, path), + parentId: context.chatId, provenance: receipt, - view: 'complete', + view: input.view, }) input.signal?.throwIfAborted() return { diff --git a/apps/sim/lib/mothership/content-delivery/registry-gate.test.ts b/apps/sim/lib/mothership/content-delivery/registry-gate.test.ts index ce5ca212a7a..67abfb84e8c 100644 --- a/apps/sim/lib/mothership/content-delivery/registry-gate.test.ts +++ b/apps/sim/lib/mothership/content-delivery/registry-gate.test.ts @@ -121,7 +121,7 @@ describe('createRegistryContentGate', () => { it('refuses an opaque view of an item holding secrets, which no redaction reaches', async () => { const registry = newRegistry() const gate = createRegistryContentGate({ registry, actorUserId: 'user-1' }) - expect(await gate.admit([item('archive', exact(SECRET), 'opaque')])).toEqual([false]) + expect(await gate.admit([item('archive', exact(SECRET), 'opaque')])).toEqual(['opaque']) expect(registry.isPermanentlyIncomplete()).toBe(false) expect(telemetry.reportDurableSecretProvenanceRefusals).toHaveBeenCalledWith([ expect.objectContaining({ @@ -138,7 +138,7 @@ describe('createRegistryContentGate', () => { item(`legacy-${index}`, { status: 'unknown' }) ) const verdicts = await gate.admit([...refused, item('clean', exact())]) - expect(verdicts).toEqual([...refused.map(() => false), true]) + expect(verdicts).toEqual([...refused.map(() => 'unknown'), true]) expect(telemetry.reportDurableSecretProvenanceRefusals).toHaveBeenCalledTimes(1) const [reports] = telemetry.reportDurableSecretProvenanceRefusals.mock.calls[0] expect(reports).toHaveLength(50) @@ -147,7 +147,7 @@ describe('createRegistryContentGate', () => { it('refuses an item holding secrets when there is no registry to redact with', async () => { const gate = createRegistryContentGate({ actorUserId: 'user-1' }) - expect(await gate.admit([item('config', exact(SECRET))])).toEqual([false]) + expect(await gate.admit([item('config', exact(SECRET))])).toEqual(['registry']) }) it('tracks opaque workbench downloads without relaxing model or excerpt admission', async () => { @@ -159,18 +159,18 @@ describe('createRegistryContentGate', () => { }) expect(await gate.admit([item('download', exact(SECRET), 'opaque')])).toEqual([true]) expect(modelView(SECRET, registry)).not.toContain(SECRET) - expect(await gate.admit([item('snippet', exact(SECRET), 'excerpt')])).toEqual([false]) + expect(await gate.admit([item('snippet', exact(SECRET), 'excerpt')])).toEqual(['opaque']) expect( await createRegistryContentGate({ actorUserId: 'user-1', destination: 'workbench' }).admit([ item('download', exact(SECRET), 'opaque'), ]) - ).toEqual([false]) + ).toEqual(['registry']) }) it('refuses an unknown item without latching the registry', async () => { const registry = newRegistry() const gate = createRegistryContentGate({ registry, actorUserId: 'user-1' }) - expect(await gate.admit([item('tainted', { status: 'unknown' })])).toEqual([false]) + expect(await gate.admit([item('tainted', { status: 'unknown' })])).toEqual(['unknown']) expect(registry.isPermanentlyIncomplete()).toBe(false) }) @@ -184,7 +184,7 @@ describe('createRegistryContentGate', () => { it('refuses an unknown item under taint when no registry could withhold the output', async () => { const gate = createRegistryContentGate({ actorUserId: 'user-1', unknown: 'taint' }) - expect(await gate.admit([item('tainted', { status: 'unknown' })])).toEqual([false]) + expect(await gate.admit([item('tainted', { status: 'unknown' })])).toEqual(['unknown']) }) it('refuses only the item whose secrets cannot be imported, keeping the rest of the call', async () => { @@ -199,7 +199,7 @@ describe('createRegistryContentGate', () => { item('config', exact(SECRET)), item('clean', { status: 'exact', entries: [] }), ]) - expect(verdicts).toEqual([false, true, true]) + expect(verdicts).toEqual(['import', true, true]) expect(registry.isPermanentlyIncomplete()).toBe(false) expect(modelView(`token=${SECRET}`, registry)).not.toContain(SECRET) }) @@ -213,7 +213,7 @@ describe('createRegistryContentGate', () => { const gate = createRegistryContentGate({ registry, actorUserId: 'user-1' }) expect(await gate.admit([item('existing', exact(...values))])).toEqual([true]) - expect(await gate.admit([item('overflow', exact(SECRET, OTHER_SECRET))])).toEqual([false]) + expect(await gate.admit([item('overflow', exact(SECRET, OTHER_SECRET))])).toEqual(['import']) expect(registry.isPermanentlyIncomplete()).toBe(false) expect(modelView(`public notes token=${values[0]}`, registry)).toBe( @@ -236,7 +236,7 @@ describe('createRegistryContentGate', () => { gate.admit([item('second', exact(OTHER_SECRET))]), ]) - expect(verdicts.flat().sort()).toEqual([false, true]) + expect(verdicts.flat().sort()).toEqual(['import', true]) expect(registry.isPermanentlyIncomplete()).toBe(false) expect(modelView(`public notes token=${values[0]}`, registry)).toBe( 'public notes token={{FIXTURE_TOKEN}}' diff --git a/apps/sim/lib/mothership/content-delivery/registry-gate.ts b/apps/sim/lib/mothership/content-delivery/registry-gate.ts index bdd66148c83..3937b3d1ae2 100644 --- a/apps/sim/lib/mothership/content-delivery/registry-gate.ts +++ b/apps/sim/lib/mothership/content-delivery/registry-gate.ts @@ -1,4 +1,4 @@ -import type { ContentDeliveryGate, ContentSource } from '@/lib/core/application' +import type { ContentDeliveryGate, ContentSource, ContentVerdict } from '@/lib/core/application' import { importDurableSecretProvenance } from '@/lib/execution/durable-secret-provenance' import { type DurableSecretProvenanceUnrecordedReport, @@ -37,7 +37,7 @@ export function createRegistryContentGate(scope: RegistryContentGateScope): Cont source: ContentSource, unrecorded: DurableSecretProvenanceUnrecordedReport[], { refuse }: ContentRefusals - ): Promise => { + ): Promise => { const { provenance } = source if (provenance.status === 'unknown') { /** Taint needs a registry to latch; without one the bytes would reach the model as is. */ @@ -88,7 +88,7 @@ export function createRegistryContentGate(scope: RegistryContentGateScope): Cont async admit(sources) { const unrecorded: DurableSecretProvenanceUnrecordedReport[] = [] const refusals = createContentRefusals(scope.workspaceId) - const verdicts: boolean[] = [] + const verdicts: ContentVerdict[] = [] try { for (const source of sources) verdicts.push(await decide(source, unrecorded, refusals)) } finally { diff --git a/apps/sim/lib/mothership/content-delivery/session-gate.test.ts b/apps/sim/lib/mothership/content-delivery/session-gate.test.ts index 6b6d74e8298..6b664b7c48f 100644 --- a/apps/sim/lib/mothership/content-delivery/session-gate.test.ts +++ b/apps/sim/lib/mothership/content-delivery/session-gate.test.ts @@ -43,7 +43,7 @@ beforeEach(() => { describe('createSessionContentGate', () => { it('keeps an unknown item out of the machine and its history', async () => { - expect(await gate().admit([item('tainted', { status: 'unknown' })])).toEqual([false]) + expect(await gate().admit([item('tainted', { status: 'unknown' })])).toEqual(['unknown']) expect(history.recordExistingSessionFileInput).not.toHaveBeenCalled() expect(telemetry.reportDurableSecretProvenanceRefusals).toHaveBeenCalledWith([ expect.objectContaining({ resourceId: 'tainted' }), @@ -55,7 +55,7 @@ describe('createSessionContentGate', () => { const refused = Array.from({ length: 50 }, (_, index) => item(`tainted-${index}`, { status: 'unknown' }) ) - expect(await gate().admit(refused)).toEqual(refused.map(() => false)) + expect(await gate().admit(refused)).toEqual(refused.map(() => 'unknown')) expect(telemetry.reportDurableSecretProvenanceRefusals).toHaveBeenCalledTimes(1) const [reports] = telemetry.reportDurableSecretProvenanceRefusals.mock.calls[0] expect(reports).toHaveLength(50) @@ -68,7 +68,7 @@ describe('createSessionContentGate', () => { item('tainted', { status: 'unknown' }), item('clean', { status: 'exact', entries: [] }), ]) - expect(verdicts).toEqual([true, false, true]) + expect(verdicts).toEqual([true, 'unknown', true]) expect(history.recordExistingSessionFileInput).toHaveBeenCalledTimes(1) expect(history.recordExistingSessionFileInput).toHaveBeenCalledWith('session-1', { status: 'exact', diff --git a/apps/sim/lib/mothership/content-delivery/session-gate.ts b/apps/sim/lib/mothership/content-delivery/session-gate.ts index 5e7e89eafe7..89f6b1c439e 100644 --- a/apps/sim/lib/mothership/content-delivery/session-gate.ts +++ b/apps/sim/lib/mothership/content-delivery/session-gate.ts @@ -1,4 +1,4 @@ -import type { ContentDeliveryGate } from '@/lib/core/application' +import type { ContentDeliveryGate, ContentVerdict } from '@/lib/core/application' import { createDurableSecretProvenanceRegistry, type DurableSecretProvenance, @@ -53,7 +53,7 @@ export function createSessionContentGate(scope: { } }, async admit(sources) { - const verdicts: boolean[] = [] + const verdicts: ContentVerdict[] = [] const admitted: DurableSecretProvenance[] = [] const refusals = createContentRefusals(scope.workspaceId) let unrecorded = 0 diff --git a/apps/sim/lib/mothership/content-delivery/telemetry.test.ts b/apps/sim/lib/mothership/content-delivery/telemetry.test.ts new file mode 100644 index 00000000000..b70a864cbf7 --- /dev/null +++ b/apps/sim/lib/mothership/content-delivery/telemetry.test.ts @@ -0,0 +1,40 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import type { ContentRefusalReason, ContentSourceKind } from '@/lib/core/application' +import { describeWithheldContent } from '@/lib/mothership/content-delivery/telemetry' + +function withheld( + entries: [ContentSourceKind, [ContentRefusalReason, number][]][] +): Map> { + return new Map(entries.map(([kind, reasons]) => [kind, new Map(reasons)])) +} + +describe('describeWithheldContent', () => { + it('says nothing when nothing was withheld', () => { + expect(describeWithheldContent(new Map())).toBeUndefined() + }) + + it('tells Chat what each refusal means for its next read, naming no item', () => { + const note = describeWithheldContent( + withheld([ + ['knowledge-document', [['unknown', 3]]], + ['workspace-file', [['opaque', 1]]], + [ + 'table-row', + [ + ['registry', 2], + ['import', 1], + ], + ], + ]) + ) + expect(note?.split('\n')).toEqual([ + 'Note: 3 knowledge documents were withheld from this result: their secret provenance is unknown, so reading them again will not return them. They exist; do not treat them as missing.', + 'Note: 1 file was withheld from this result: it contains recorded secrets and cannot be shown as raw bytes or a preview. Read it as text for a redacted copy.', + 'Note: 2 table rows were withheld from this result: their secret provenance could not be verified right now. They may be readable on a later request.', + 'Note: 1 table row was withheld from this result: its secret provenance could not be verified right now. It may be readable on a later request.', + ]) + }) +}) diff --git a/apps/sim/lib/mothership/content-delivery/telemetry.ts b/apps/sim/lib/mothership/content-delivery/telemetry.ts index e7662d30040..4c0597d0741 100644 --- a/apps/sim/lib/mothership/content-delivery/telemetry.ts +++ b/apps/sim/lib/mothership/content-delivery/telemetry.ts @@ -1,4 +1,9 @@ -import type { ContentSource, ContentSourceKind } from '@/lib/core/application' +import type { + ContentRefusalReason, + ContentSource, + ContentSourceKind, + WithheldContent, +} from '@/lib/core/application' import { type DurableSecretProvenanceRefusalCause, type DurableSecretProvenanceRefusalReport, @@ -6,8 +11,6 @@ import { reportDurableSecretProvenanceRefusals, } from '@/lib/execution/durable-secret-provenance-telemetry' -type ContentRefusalReason = 'unknown' | 'opaque' | 'registry' | 'import' - export const CONTENT_SURFACE: Record = { 'workspace-file': 'workspace-file', 'execution-file': 'workspace-file', @@ -57,27 +60,28 @@ const CAUSES: Record< } /** - * Collects the items one admission refuses and reports them once, naming each item and why, never - * its content. A page of legacy rows is one line, not one per row. + * Collects the items one admission refuses and reports them once per surface and cause, naming + * each item by its opaque id and why, never its content. A page of legacy rows is one line. */ export interface ContentRefusals { - /** Records the refusal of `source`; returns the verdict, `false`. */ - refuse(source: ContentSource, reason: ContentRefusalReason): false - /** Reports what was recorded, as one line. */ + /** Records the refusal of `source`; returns the verdict, its reason. */ + refuse(source: ContentSource, reason: ContentRefusalReason): ContentRefusalReason + /** Reports what was recorded. */ report(): void } export function createContentRefusals(workspaceId: string | undefined): ContentRefusals { const reports: DurableSecretProvenanceRefusalReport[] = [] return { - refuse(source: ContentSource, reason: ContentRefusalReason): false { + refuse(source, reason) { reports.push({ surface: CONTENT_SURFACE[source.kind], cause: CAUSES[source.kind][reason], workspaceId, resourceId: source.id, + ...(source.parentId ? { parentId: source.parentId } : {}), }) - return false + return reason }, report(): void { reportDurableSecretProvenanceRefusals(reports) @@ -94,20 +98,40 @@ const WITHHELD_LABELS: Record = { 'tool-result': ['tool result', 'tool results'], } +/** What each refusal means for Chat's next read, as `[one item, several items]`. */ +const WITHHELD_REASONS: Record = { + unknown: [ + 'its secret provenance is unknown, so reading it again will not return it. It exists; do not treat it as missing.', + 'their secret provenance is unknown, so reading them again will not return them. They exist; do not treat them as missing.', + ], + opaque: [ + 'it contains recorded secrets and cannot be shown as raw bytes or a preview. Read it as text for a redacted copy.', + 'they contain recorded secrets and cannot be shown as raw bytes or a preview. Read them as text for a redacted copy.', + ], + registry: [ + 'its secret provenance could not be verified right now. It may be readable on a later request.', + 'their secret provenance could not be verified right now. They may be readable on a later request.', + ], + import: [ + 'its secret provenance could not be verified right now. It may be readable on a later request.', + 'their secret provenance could not be verified right now. They may be readable on a later request.', + ], +} + /** - * Tells Chat's model what a result left out: a refused item is dropped from the page it was read - * in, which otherwise reads exactly like an item that does not exist. A refusal can be of the - * item (its provenance is unknown) or only of the view (an excerpt or raw bytes redaction cannot - * cover), so the note says it exists and claims nothing about other reads. + * Tells Chat's model what a result left out, and what to do about it: a refused item is dropped + * from the page it was read in, which otherwise reads exactly like an item that does not exist, + * and a model that cannot tell why keeps reading it again. Names no item. */ -export function describeWithheldContent( - withheld: ReadonlyMap -): string | undefined { - const lines = [...withheld].map(([kind, count]) => { - const [singular, plural] = WITHHELD_LABELS[kind] - return count === 1 - ? `Note: 1 ${singular} was withheld from this result because it could not be shown safely (its secret provenance is unknown, or this view of it cannot be redacted). It exists; do not treat it as missing.` - : `Note: ${count} ${plural} were withheld from this result because they could not be shown safely (their secret provenance is unknown, or this view of them cannot be redacted). They exist; do not treat them as missing.` - }) +export function describeWithheldContent(withheld: WithheldContent): string | undefined { + const lines: string[] = [] + for (const [kind, reasons] of withheld) { + for (const [reason, count] of reasons) { + const form = count === 1 ? 0 : 1 + lines.push( + `Note: ${count} ${WITHHELD_LABELS[kind][form]} ${count === 1 ? 'was' : 'were'} withheld from this result: ${WITHHELD_REASONS[reason][form]}` + ) + } + } return lines.length > 0 ? lines.join('\n') : undefined } diff --git a/apps/sim/lib/table/application/rows.test.ts b/apps/sim/lib/table/application/rows.test.ts index ba6f8635345..c7fa324dddf 100644 --- a/apps/sim/lib/table/application/rows.test.ts +++ b/apps/sim/lib/table/application/rows.test.ts @@ -950,7 +950,13 @@ describe('row delivery to Chat', () => { expect(mockRowProvenance).toHaveBeenCalledWith(ROW.id) expect(admit).toHaveBeenCalledTimes(1) expect(admit).toHaveBeenCalledWith([ - { kind: 'table-row', id: ROW.id, provenance, view: { derived: ROW.data } }, + { + kind: 'table-row', + id: ROW.id, + parentId: TABLE.id, + provenance, + view: { derived: ROW.data }, + }, ]) }) diff --git a/apps/sim/lib/table/application/rows.ts b/apps/sim/lib/table/application/rows.ts index b10607ebbbc..a216b74ef55 100644 --- a/apps/sim/lib/table/application/rows.ts +++ b/apps/sim/lib/table/application/rows.ts @@ -171,7 +171,7 @@ async function deliverRows( principal: Principal, reader: TableRowProvenanceReader | undefined, rows: readonly R[], - options: { written?: boolean } = {} + options: { tableId: string; written?: boolean } ): Promise { if (!reader || !mustClassifyContent(principal)) { await admitContent(principal, []) @@ -180,6 +180,7 @@ async function deliverRows( const sources = rows.map((row) => ({ kind: 'table-row' as const, id: row.id, + parentId: options.tableId, provenance: reader.rowProvenance(row.id), view: { derived: row.data }, })) @@ -522,7 +523,9 @@ export const listTableRows = defineAuthorizedTableUseCase({ requestId(input), readProvenance ) - const rows = await deliverRows(principal, readProvenance, result.rows) + const rows = await deliverRows(principal, readProvenance, result.rows, { + tableId: context.table.id, + }) return { table: context.table, rows: rowsForReader(principal, rows), @@ -647,7 +650,9 @@ export const queryTableRows = defineAuthorizedTableUseCase({ requestId(input), readProvenance ) - const rows = await deliverRows(principal, readProvenance, result.rows) + const rows = await deliverRows(principal, readProvenance, result.rows, { + tableId: context.table.id, + }) return { table: context.table, ...result, @@ -738,7 +743,10 @@ export const readTableRow = defineAuthorizedTableUseCase({ budgetBytes: TABLE_LIMITS.MAX_ROW_RUN_STATE_BYTES, }) : undefined - if ((await deliverRows(principal, readProvenance, [row])).length === 0) { + if ( + (await deliverRows(principal, readProvenance, [row], { tableId: context.table.id })) + .length === 0 + ) { throw new ContentRefusedError('table-row') } return { @@ -807,7 +815,10 @@ export const readTableRowEnrichmentDetail = defineAuthorizedTableUseCase({ loadEnrichmentDetail(db, context.tableId, input.rowId, input.groupId), ]) const runState = executions[input.groupId] ?? null - if ((await deliverRows(principal, readProvenance, [row])).length === 0) { + if ( + (await deliverRows(principal, readProvenance, [row], { tableId: context.table.id })) + .length === 0 + ) { throw new ContentRefusedError('table-row') } return { @@ -910,7 +921,10 @@ export const createTableRows = defineAuthorizedTableUseCase({ requestId(input), writeOptions ) - await deliverRows(principal, readProvenance, [row], { written: true }) + await deliverRows(principal, readProvenance, [row], { + tableId: context.table.id, + written: true, + }) return { kind: 'single', table: context.table, @@ -966,7 +980,10 @@ export const createTableRows = defineAuthorizedTableUseCase({ requestId(input), batchWriteOptions ) - await deliverRows(principal, readProvenance, created, { written: true }) + await deliverRows(principal, readProvenance, created, { + tableId: context.table.id, + written: true, + }) return { kind: 'batch', table: context.table, @@ -1306,7 +1323,10 @@ export const updateTableRow = defineAuthorizedTableUseCase({ { ...rowWriteOptions(input), readProvenance } ) if (!row) throw new Error('Unconditional table row update was rejected') - await deliverRows(principal, readProvenance, [row], { written: true }) + await deliverRows(principal, readProvenance, [row], { + tableId: context.table.id, + written: true, + }) return { table: context.table, row, @@ -1673,7 +1693,10 @@ export const upsertTableRow = defineAuthorizedTableUseCase({ requestId(input), { ...rowWriteOptions(input), readProvenance } ) - await deliverRows(principal, readProvenance, [result.row], { written: true }) + await deliverRows(principal, readProvenance, [result.row], { + tableId: context.table.id, + written: true, + }) return { table: context.table, row: result.row, From 916bbda96413c02868007d849eb9652eb5585721 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 15 Oct 2026 17:03:34 -0700 Subject: [PATCH 5/7] fix(billing): an in-flight refusal is not a slow ledger transaction The usage ledger's slow-transaction warning fired for every in-flight refusal (stage lock, not succeeded), although nothing waited and the route already logs the refusal once at INFO. It now skips refusals. The span keeps the existing internal_error outcome, since the closed outcome set is generated from the legacy contracts; the response code and the INFO line tell it apart. --- apps/sim/app/api/billing/update-cost/route.ts | 2 ++ apps/sim/lib/billing/core/usage-log.test.ts | 6 ++++++ apps/sim/lib/billing/core/usage-log.ts | 5 ++++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/api/billing/update-cost/route.ts b/apps/sim/app/api/billing/update-cost/route.ts index 8857cec569e..f67c5dd2ff4 100644 --- a/apps/sim/app/api/billing/update-cost/route.ts +++ b/apps/sim/app/api/billing/update-cost/route.ts @@ -521,6 +521,8 @@ async function updateCostInner(req: NextRequest, span: Span): Promise { expect(select).not.toHaveBeenCalled() expect(mockInsert).not.toHaveBeenCalled() expect(mockUpdate).not.toHaveBeenCalled() + /** The route logs the refusal once; it is not a transaction that failed to complete. */ + expect(getMockLogger('UsageLog').warn).not.toHaveBeenCalledWith( + 'Cumulative usage transaction did not complete promptly', + expect.anything() + ) }) it('tops up to the higher cumulative and bills only the delta', async () => { diff --git a/apps/sim/lib/billing/core/usage-log.ts b/apps/sim/lib/billing/core/usage-log.ts index 44004996ad1..b2130f361db 100644 --- a/apps/sim/lib/billing/core/usage-log.ts +++ b/apps/sim/lib/billing/core/usage-log.ts @@ -832,6 +832,7 @@ export async function recordCumulativeUsage( const stageDurationsMs: Partial> = {} let succeeded = false let pgCode: string | undefined + let inFlight = false let toppedUpRowCreatedAt: Date | undefined const enterStage = (nextStage: CumulativeUsageStage) => { const now = Date.now() @@ -1015,12 +1016,14 @@ export async function recordCumulativeUsage( return result } catch (error) { pgCode = getPostgresErrorCode(error) + /** Another attempt holding the lock is an answer, not a slow transaction; the route logs it. */ + if (error instanceof CumulativeUsageInFlightError) inFlight = true throw error } finally { const now = Date.now() stageDurationsMs[stage] = now - stageStartedAt const durationMs = now - startedAt - if (!succeeded || durationMs >= CUMULATIVE_FLUSH_SLOW_MS) { + if (!inFlight && (!succeeded || durationMs >= CUMULATIVE_FLUSH_SLOW_MS)) { logger.warn('Cumulative usage transaction did not complete promptly', { eventKey, succeeded, From 01c35e445e05e4ce98d5e565c7af14351cc6378b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 15 Oct 2026 17:07:21 -0700 Subject: [PATCH 6/7] fix(provenance): keep this change to the refusal logs and note it introduced Drops the sandbox attribution id, the streamed-image view and the owning-record ids from this change; they belong to the provenance owner's work and fix behavior that predates it. Refusal lines stay one per surface and cause with top-level surface, cause and refusedCount and every refused id as each line names it today, and the note keeps its per-reason wording. --- apps/sim/lib/billing/core/usage-log.test.ts | 2 +- apps/sim/lib/billing/core/usage-log.ts | 2 +- .../lib/core/application/content-delivery.ts | 4 +- ...urable-secret-provenance-telemetry.test.ts | 6 +- .../durable-secret-provenance-telemetry.ts | 11 +--- .../application/admit-knowledge-content.ts | 1 - apps/sim/lib/knowledge/application/chunks.ts | 4 +- .../agent-cli/engines/scratch-file-read.ts | 1 - .../application/inline-images-scratch.test.ts | 59 +------------------ .../chat/application/inline-images.ts | 2 +- .../application/read-sandbox-file.test.ts | 7 +-- .../chat/application/read-sandbox-file.ts | 16 +---- .../mothership/content-delivery/telemetry.ts | 3 +- apps/sim/lib/table/application/rows.test.ts | 8 +-- apps/sim/lib/table/application/rows.ts | 41 +++---------- 15 files changed, 26 insertions(+), 141 deletions(-) diff --git a/apps/sim/lib/billing/core/usage-log.test.ts b/apps/sim/lib/billing/core/usage-log.test.ts index 09d0a9da3be..c40202faa5f 100644 --- a/apps/sim/lib/billing/core/usage-log.test.ts +++ b/apps/sim/lib/billing/core/usage-log.test.ts @@ -308,7 +308,7 @@ describe('recordCumulativeUsage', () => { expect(select).not.toHaveBeenCalled() expect(mockInsert).not.toHaveBeenCalled() expect(mockUpdate).not.toHaveBeenCalled() - /** The route logs the refusal once; it is not a transaction that failed to complete. */ + // The route logs the refusal once; it is not a transaction that failed to complete. expect(getMockLogger('UsageLog').warn).not.toHaveBeenCalledWith( 'Cumulative usage transaction did not complete promptly', expect.anything() diff --git a/apps/sim/lib/billing/core/usage-log.ts b/apps/sim/lib/billing/core/usage-log.ts index b2130f361db..2c29a25e4f1 100644 --- a/apps/sim/lib/billing/core/usage-log.ts +++ b/apps/sim/lib/billing/core/usage-log.ts @@ -1016,7 +1016,7 @@ export async function recordCumulativeUsage( return result } catch (error) { pgCode = getPostgresErrorCode(error) - /** Another attempt holding the lock is an answer, not a slow transaction; the route logs it. */ + // Another attempt holding the lock is an answer, not a slow transaction; the route logs it. if (error instanceof CumulativeUsageInFlightError) inFlight = true throw error } finally { diff --git a/apps/sim/lib/core/application/content-delivery.ts b/apps/sim/lib/core/application/content-delivery.ts index 23741c8dea1..bc5f3d52259 100644 --- a/apps/sim/lib/core/application/content-delivery.ts +++ b/apps/sim/lib/core/application/content-delivery.ts @@ -31,10 +31,8 @@ export type ContentView = 'complete' | 'opaque' | 'excerpt' | { derived: unknown /** One stored item or tool result whose content a use case is about to return. */ export interface ContentSource { kind: ContentSourceKind - /** Canonical id, for attribution and per-item exclusion only. Never a name or path. */ + /** Canonical id, for attribution and per-item exclusion only. */ id: string - /** The record that owns the item (its document, table or chat), for attribution only. */ - parentId?: string provenance: ContentProvenance view: ContentView } diff --git a/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts b/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts index 473f3941446..e861f4ada23 100644 --- a/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts +++ b/apps/sim/lib/execution/durable-secret-provenance-telemetry.test.ts @@ -53,7 +53,7 @@ describe('durable secret provenance telemetry', () => { } ) }) - it('reports a batch once per surface, cause and owner, keeping every id paired with its cause', () => { + it('reports a batch once per surface and cause, keeping every id paired with its cause', () => { logger.warn.mockClear() const reports = Array.from({ length: 163 }, (_, index) => ({ surface: 'knowledge' as const, @@ -62,7 +62,6 @@ describe('durable secret provenance telemetry', () => { ? ('knowledge-chunk-source-unavailable' as const) : ('knowledge-opaque-secret-content' as const), workspaceId: 'workspace-1', - parentId: 'document-1', resourceId: `chunk-${index}`, })) reportDurableSecretProvenanceRefusals(reports) @@ -75,7 +74,6 @@ describe('durable secret provenance telemetry', () => { cause: 'knowledge-chunk-source-unavailable', refusedCount: 160, workspaceId: 'workspace-1', - parentId: 'document-1', resourceIds: reports.slice(0, 160).map((report) => report.resourceId), } ) @@ -87,7 +85,6 @@ describe('durable secret provenance telemetry', () => { cause: 'knowledge-opaque-secret-content', refusedCount: 3, workspaceId: 'workspace-1', - parentId: 'document-1', resourceIds: ['chunk-160', 'chunk-161', 'chunk-162'], } ) @@ -100,7 +97,6 @@ describe('durable secret provenance telemetry', () => { Array.from({ length: 250 }, (_, index) => ({ surface: 'table-row' as const, cause: 'table-row-provenance-unavailable' as const, - parentId: 'table-1', resourceId: `row-${index}`, })) ) diff --git a/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts b/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts index 5f6dfa5943b..74eb8450143 100644 --- a/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts +++ b/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts @@ -50,10 +50,7 @@ export interface DurableSecretProvenanceRefusalReport { surface: DurableSecretProvenanceSurface cause: DurableSecretProvenanceRefusalCause workspaceId?: string - /** An opaque id, never a name or path. */ resourceId?: string - /** The record that owns the resource (its document, table or chat). */ - parentId?: string } /** @@ -84,16 +81,15 @@ export function reportDurableSecretProvenanceRefusal( surface: report.surface, cause: report.cause, ...(report.workspaceId ? { workspaceId: report.workspaceId } : {}), - ...(report.parentId ? { parentId: report.parentId } : {}), ...(report.resourceId ? { resourceId: report.resourceId } : {}), }) } -/** Ids one grouped refusal lists; the ids are opaque, and a page of them stays one line. */ +/** Ids one grouped refusal lists, so a page of them stays one line. */ const REFUSAL_RESOURCE_ID_LIMIT = 200 /** - * Reports the refusals of one read with one line per surface, cause and owning record, each + * Reports the refusals of one read with one line per surface and cause, each * carrying the same top-level `surface` and `cause` as a single refusal, its `refusedCount`, and * the refused ids, so each id stays paired with its cause. */ @@ -102,7 +98,7 @@ export function reportDurableSecretProvenanceRefusals( ): void { const groups = new Map() for (const report of reports) { - const key = JSON.stringify([report.surface, report.cause, report.workspaceId, report.parentId]) + const key = JSON.stringify([report.surface, report.cause, report.workspaceId]) const group = groups.get(key) if (group) group.push(report) else groups.set(key, [report]) @@ -119,7 +115,6 @@ export function reportDurableSecretProvenanceRefusals( cause: first.cause, refusedCount: group.length, ...(first.workspaceId ? { workspaceId: first.workspaceId } : {}), - ...(first.parentId ? { parentId: first.parentId } : {}), resourceIds: resourceIds.slice(0, REFUSAL_RESOURCE_ID_LIMIT), ...(resourceIds.length > REFUSAL_RESOURCE_ID_LIMIT ? { resourceIdsTruncated: true } : {}), }) diff --git a/apps/sim/lib/knowledge/application/admit-knowledge-content.ts b/apps/sim/lib/knowledge/application/admit-knowledge-content.ts index acba3188330..736322e5cdf 100644 --- a/apps/sim/lib/knowledge/application/admit-knowledge-content.ts +++ b/apps/sim/lib/knowledge/application/admit-knowledge-content.ts @@ -81,7 +81,6 @@ export async function admitKnowledgeChunks( return items.map((chunk) => ({ kind: 'knowledge-chunk' as const, id: chunk.id, - parentId: chunk.documentId, provenance: provenance.get(chunk.id) ?? { status: 'unknown' as const }, view: chunk.value === undefined ? ('complete' as const) : { derived: chunk.value }, })) diff --git a/apps/sim/lib/knowledge/application/chunks.ts b/apps/sim/lib/knowledge/application/chunks.ts index f55469b2787..719f87d56ed 100644 --- a/apps/sim/lib/knowledge/application/chunks.ts +++ b/apps/sim/lib/knowledge/application/chunks.ts @@ -212,7 +212,7 @@ export const createKnowledgeChunk = defineAuthorizedKnowledgeUseCase({ surface: 'knowledge', cause: 'knowledge-chunk-source-unavailable', workspaceId: context.workspaceId, - parentId: context.documentId, + resourceId: context.documentId, }) throw new OrchestrationError('validation', 'Knowledge chunk secret provenance is unavailable') } @@ -288,7 +288,7 @@ export const updateKnowledgeChunk = defineAuthorizedKnowledgeUseCase({ surface: 'knowledge', cause: 'knowledge-chunk-source-unavailable', workspaceId: context.workspaceId, - parentId: context.documentId, + resourceId: context.documentId, }) throw new OrchestrationError('validation', 'Knowledge chunk secret provenance is unavailable') } diff --git a/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts b/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts index e4590cadeeb..accf2cd0553 100644 --- a/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts +++ b/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts @@ -20,7 +20,6 @@ export async function readScratchFile( ? { organizationId: runtime.chatOrganizationId } : { workspaceId: runtime.workspaceId }), path: reference, - view: 'complete', maxBytes: options.maxBytes, signal: runtime.signal, }, diff --git a/apps/sim/lib/mothership/chat/application/inline-images-scratch.test.ts b/apps/sim/lib/mothership/chat/application/inline-images-scratch.test.ts index 967e60eda14..11c5afbc976 100644 --- a/apps/sim/lib/mothership/chat/application/inline-images-scratch.test.ts +++ b/apps/sim/lib/mothership/chat/application/inline-images-scratch.test.ts @@ -1,7 +1,6 @@ import { copilotChats } from '@sim/db/schema' import { queueTableRows, resetDbChainMock } from '@sim/testing' import { authBanMock } from '@sim/testing/mocks/auth-ban.mock' -import { getMockLogger } from '@sim/testing/mocks/logger.mock' import { remoteSandboxMock } from '@sim/testing/mocks/remote-sandbox.mock' import { storageServiceMock, storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock' import { workspaceAuthzMock, workspaceAuthzMockFns } from '@sim/testing/mocks/workspace-authz.mock' @@ -9,11 +8,7 @@ import { workspaceContextMock } from '@sim/testing/mocks/workspace-context.mock' import sharp from 'sharp' import { beforeEach, expect, it, vi } from 'vitest' -const hoisted = vi.hoisted(() => ({ - snapshot: vi.fn(), - dispose: vi.fn(), - provenance: vi.fn(() => ({ status: 'exact', entries: [] as unknown[] })), -})) +const hoisted = vi.hoisted(() => ({ snapshot: vi.fn(), dispose: vi.fn() })) vi.mock('@sim/platform-authz/workspace', () => workspaceAuthzMock) vi.mock('@/lib/auth/ban', () => authBanMock) vi.mock('@/lib/workspaces/application/context', () => workspaceContextMock) @@ -30,7 +25,7 @@ vi.mock('@/lib/execution/remote-sandbox/session-file-snapshot', () => ({ vi.mock('@/lib/mothership/agent-cli/workbench-file-provenance', () => ({ createWorkbenchFileProvenance: () => ({ observeUpload: (_machine: unknown, stream: unknown) => stream, - uploadProvenance: () => hoisted.provenance(), + uploadProvenance: () => ({ status: 'exact', entries: [] }), }), })) vi.mock('@/lib/execution/remote-sandbox', () => remoteSandboxMock) @@ -88,53 +83,3 @@ it('runs the real nested scratch authorization, limit validation, snapshot read expect.objectContaining({ createOnly: true, persistMetadata: false, contentType: 'image/webp' }) ) }) - -it('refuses an image holding recorded secrets as opaque, naming it without its path', async () => { - const owner = { - userId: 'owner', - workspaceId: 'workspace', - organizationId: null, - type: 'mothership', - } - queueTableRows(copilotChats, [owner]) - queueTableRows(copilotChats, [owner]) - hoisted.provenance.mockReturnValueOnce({ - status: 'exact', - entries: [ - { - encryptedValue: 'ciphertext', - sourceUserId: 'owner', - sourceWorkspaceId: 'workspace', - name: 'FIXTURE_TOKEN', - }, - ], - }) - const buffer = Buffer.from('not shown') - mocks.snapshot.mockResolvedValue({ - size: buffer.length, - dispose: mocks.dispose, - stream: async () => - new ReadableStream({ - start(controller) { - controller.enqueue(buffer) - controller.close() - }, - }), - }) - const refusals = getMockLogger('DurableSecretProvenancePersistence').warn - refusals.mockClear() - await expect( - materializeStreamImage( - { userId: 'owner', workspaceId: 'workspace', chatId: 'chat' }, - { requestId: 'request-secret', reference: '/tmp/quarterly%20payroll.png' } - ) - ).rejects.toThrow() - expect(refusals).toHaveBeenCalledTimes(1) - const [, fields] = refusals.mock.calls[0] - expect(fields).toMatchObject({ - surface: 'workspace-file', - cause: 'workspace-file-opaque-secret-content', - parentId: 'chat', - }) - expect(JSON.stringify(fields)).not.toMatch(/payroll|\/tmp/) -}) diff --git a/apps/sim/lib/mothership/chat/application/inline-images.ts b/apps/sim/lib/mothership/chat/application/inline-images.ts index d0aeac1af60..2c1dd3f388d 100644 --- a/apps/sim/lib/mothership/chat/application/inline-images.ts +++ b/apps/sim/lib/mothership/chat/application/inline-images.ts @@ -140,7 +140,7 @@ export const materializeInlineChatImage = defineAuthorizedChatUseCase({ source = ( await readChatSandboxFile.execute({ principal, - input: { ...scope, path: decodeURIComponent(reference), view: 'opaque', signal }, + input: { ...scope, path: decodeURIComponent(reference), signal }, request, }) ).buffer diff --git a/apps/sim/lib/mothership/chat/application/read-sandbox-file.test.ts b/apps/sim/lib/mothership/chat/application/read-sandbox-file.test.ts index 41173657785..edf57de8da5 100644 --- a/apps/sim/lib/mothership/chat/application/read-sandbox-file.test.ts +++ b/apps/sim/lib/mothership/chat/application/read-sandbox-file.test.ts @@ -56,12 +56,7 @@ function readAsChat(scope: { principal?: Principal } = {}) { () => readChatSandboxFile.execute({ principal: scope.principal ?? chatPrincipal, input }) ) } -const input = { - workspaceId: 'ws', - chatId: 'chat', - path: '/tmp/image.png', - view: 'complete' as const, -} +const input = { workspaceId: 'ws', chatId: 'chat', path: '/tmp/image.png' } const machine = { providerId: 'e2b', sandboxId: 'physical' } beforeEach(() => { mocks.permission.mockResolvedValue('read') diff --git a/apps/sim/lib/mothership/chat/application/read-sandbox-file.ts b/apps/sim/lib/mothership/chat/application/read-sandbox-file.ts index 3b020919ba6..f31b9a86c1b 100644 --- a/apps/sim/lib/mothership/chat/application/read-sandbox-file.ts +++ b/apps/sim/lib/mothership/chat/application/read-sandbox-file.ts @@ -1,6 +1,5 @@ import { posix } from 'node:path' import type { Principal } from '@sim/auth/principal' -import { sha256Hex } from '@sim/security/hash' import { defineWorkspaceOperation, requireAdmittedContent } from '@/lib/core/application' import { defineOrganizationOperation } from '@/lib/core/application/organization-operation' import { OrchestrationError } from '@/lib/core/orchestration/types' @@ -33,19 +32,9 @@ interface ReadSandboxFileInput { organizationId?: string path: string maxBytes?: number - /** How the bytes leave: as text redaction can cover, or `opaque` (an image) it cannot reach. */ - view: 'complete' | 'opaque' signal?: AbortSignal } -/** - * Names a sandbox file in provenance attribution without its path, which carries the user's file - * name: stable for one path in one chat, and meaningless outside it. - */ -function sandboxFileAttributionId(chatId: string, path: string): string { - return `sandbox:${sha256Hex(`${chatId}\0${path}`).slice(0, 32)}` -} - /** Reads a fixed snapshot from the caller's existing private chat machine, never server disk. */ export const readChatSandboxFile = defineAuthorizedChatUseCase({ operation: defineWorkspaceOperation({ @@ -125,10 +114,9 @@ export const readChatSandboxFile = defineAuthorizedChatUseCase({ const receipt = provenance.uploadProvenance() await requireAdmittedContent(principal, { kind: 'workspace-file', - id: sandboxFileAttributionId(context.chatId, path), - parentId: context.chatId, + id: path, provenance: receipt, - view: input.view, + view: 'complete', }) input.signal?.throwIfAborted() return { diff --git a/apps/sim/lib/mothership/content-delivery/telemetry.ts b/apps/sim/lib/mothership/content-delivery/telemetry.ts index 4c0597d0741..d3946026e40 100644 --- a/apps/sim/lib/mothership/content-delivery/telemetry.ts +++ b/apps/sim/lib/mothership/content-delivery/telemetry.ts @@ -61,7 +61,7 @@ const CAUSES: Record< /** * Collects the items one admission refuses and reports them once per surface and cause, naming - * each item by its opaque id and why, never its content. A page of legacy rows is one line. + * each item by its id and why, never its content. A page of legacy rows is one line. */ export interface ContentRefusals { /** Records the refusal of `source`; returns the verdict, its reason. */ @@ -79,7 +79,6 @@ export function createContentRefusals(workspaceId: string | undefined): ContentR cause: CAUSES[source.kind][reason], workspaceId, resourceId: source.id, - ...(source.parentId ? { parentId: source.parentId } : {}), }) return reason }, diff --git a/apps/sim/lib/table/application/rows.test.ts b/apps/sim/lib/table/application/rows.test.ts index c7fa324dddf..ba6f8635345 100644 --- a/apps/sim/lib/table/application/rows.test.ts +++ b/apps/sim/lib/table/application/rows.test.ts @@ -950,13 +950,7 @@ describe('row delivery to Chat', () => { expect(mockRowProvenance).toHaveBeenCalledWith(ROW.id) expect(admit).toHaveBeenCalledTimes(1) expect(admit).toHaveBeenCalledWith([ - { - kind: 'table-row', - id: ROW.id, - parentId: TABLE.id, - provenance, - view: { derived: ROW.data }, - }, + { kind: 'table-row', id: ROW.id, provenance, view: { derived: ROW.data } }, ]) }) diff --git a/apps/sim/lib/table/application/rows.ts b/apps/sim/lib/table/application/rows.ts index a216b74ef55..b10607ebbbc 100644 --- a/apps/sim/lib/table/application/rows.ts +++ b/apps/sim/lib/table/application/rows.ts @@ -171,7 +171,7 @@ async function deliverRows( principal: Principal, reader: TableRowProvenanceReader | undefined, rows: readonly R[], - options: { tableId: string; written?: boolean } + options: { written?: boolean } = {} ): Promise { if (!reader || !mustClassifyContent(principal)) { await admitContent(principal, []) @@ -180,7 +180,6 @@ async function deliverRows( const sources = rows.map((row) => ({ kind: 'table-row' as const, id: row.id, - parentId: options.tableId, provenance: reader.rowProvenance(row.id), view: { derived: row.data }, })) @@ -523,9 +522,7 @@ export const listTableRows = defineAuthorizedTableUseCase({ requestId(input), readProvenance ) - const rows = await deliverRows(principal, readProvenance, result.rows, { - tableId: context.table.id, - }) + const rows = await deliverRows(principal, readProvenance, result.rows) return { table: context.table, rows: rowsForReader(principal, rows), @@ -650,9 +647,7 @@ export const queryTableRows = defineAuthorizedTableUseCase({ requestId(input), readProvenance ) - const rows = await deliverRows(principal, readProvenance, result.rows, { - tableId: context.table.id, - }) + const rows = await deliverRows(principal, readProvenance, result.rows) return { table: context.table, ...result, @@ -743,10 +738,7 @@ export const readTableRow = defineAuthorizedTableUseCase({ budgetBytes: TABLE_LIMITS.MAX_ROW_RUN_STATE_BYTES, }) : undefined - if ( - (await deliverRows(principal, readProvenance, [row], { tableId: context.table.id })) - .length === 0 - ) { + if ((await deliverRows(principal, readProvenance, [row])).length === 0) { throw new ContentRefusedError('table-row') } return { @@ -815,10 +807,7 @@ export const readTableRowEnrichmentDetail = defineAuthorizedTableUseCase({ loadEnrichmentDetail(db, context.tableId, input.rowId, input.groupId), ]) const runState = executions[input.groupId] ?? null - if ( - (await deliverRows(principal, readProvenance, [row], { tableId: context.table.id })) - .length === 0 - ) { + if ((await deliverRows(principal, readProvenance, [row])).length === 0) { throw new ContentRefusedError('table-row') } return { @@ -921,10 +910,7 @@ export const createTableRows = defineAuthorizedTableUseCase({ requestId(input), writeOptions ) - await deliverRows(principal, readProvenance, [row], { - tableId: context.table.id, - written: true, - }) + await deliverRows(principal, readProvenance, [row], { written: true }) return { kind: 'single', table: context.table, @@ -980,10 +966,7 @@ export const createTableRows = defineAuthorizedTableUseCase({ requestId(input), batchWriteOptions ) - await deliverRows(principal, readProvenance, created, { - tableId: context.table.id, - written: true, - }) + await deliverRows(principal, readProvenance, created, { written: true }) return { kind: 'batch', table: context.table, @@ -1323,10 +1306,7 @@ export const updateTableRow = defineAuthorizedTableUseCase({ { ...rowWriteOptions(input), readProvenance } ) if (!row) throw new Error('Unconditional table row update was rejected') - await deliverRows(principal, readProvenance, [row], { - tableId: context.table.id, - written: true, - }) + await deliverRows(principal, readProvenance, [row], { written: true }) return { table: context.table, row, @@ -1693,10 +1673,7 @@ export const upsertTableRow = defineAuthorizedTableUseCase({ requestId(input), { ...rowWriteOptions(input), readProvenance } ) - await deliverRows(principal, readProvenance, [result.row], { - tableId: context.table.id, - written: true, - }) + await deliverRows(principal, readProvenance, [result.row], { written: true }) return { table: context.table, row: result.row, From 62aae616abb3da76c76a6c4767a503948499624c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 15 Oct 2026 17:36:12 -0700 Subject: [PATCH 7/7] fix(provenance): give every refusal line a refusedCount, and say what tells an in-flight 503 apart A single refusal now carries refusedCount 1 like a grouped one, so summing refusedCount by cause covers both. The route comment now says the span is the same as the retryable settlement 503's and that the body code and the INFO line distinguish them. --- apps/sim/app/api/billing/update-cost/route.ts | 4 ++-- .../lib/execution/durable-secret-provenance-telemetry.test.ts | 1 + apps/sim/lib/execution/durable-secret-provenance-telemetry.ts | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/sim/app/api/billing/update-cost/route.ts b/apps/sim/app/api/billing/update-cost/route.ts index f67c5dd2ff4..6ff3f097f8e 100644 --- a/apps/sim/app/api/billing/update-cost/route.ts +++ b/apps/sim/app/api/billing/update-cost/route.ts @@ -521,8 +521,8 @@ async function updateCostInner(req: NextRequest, span: Span): Promise { { surface: 'workspace-file', cause: 'workspace-file-opaque-secret-content', + refusedCount: 1, workspaceId: 'workspace-1', resourceId: 'file-1', } diff --git a/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts b/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts index 74eb8450143..415665771bd 100644 --- a/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts +++ b/apps/sim/lib/execution/durable-secret-provenance-telemetry.ts @@ -80,6 +80,7 @@ export function reportDurableSecretProvenanceRefusal( persistenceLogger.warn('Refusing unavailable durable secret provenance', { surface: report.surface, cause: report.cause, + refusedCount: 1, ...(report.workspaceId ? { workspaceId: report.workspaceId } : {}), ...(report.resourceId ? { resourceId: report.resourceId } : {}), })