mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
test(table): pin the row-write behavior the suite could not fail on (#6811)
* test(table): pin the row-write behavior the suite could not fail on Mutation testing across the branch found four changes that could be reverted with the whole suite green, plus two comments asserting something untrue. - Copilot row writes now assert the translated storage keys, not the literal keying flag. The shared table fixture uses legacy columns with no id, where name-to-id mapping is the identity, so the wrong keying was unobservable; columns whose id differs from name make it fail. Copilot is the one row-write surface whose keys come from a model, and under id keying the lax write path stores those keys verbatim and reports success. - The three provenance transport helpers get direct cover. The route tests only reach mapInput and present, so each could be replaced by a constant, and a constant envelope reader silently downgrades every executor write to untracked. - The domain provenance fixtures run against the real envelope guard instead of a stub that always accepted them; none of the old fixtures was a shape the guard admits. - The attribution allowlist keys on the client-id reader rather than the field name, so it no longer misses a surface that names the acting tab positionally -- one of the two real suppliers was already invisible to it. Also restores the selectedValues narrowing on the migrated row routes, so a stale sidecar entry for a dropped column no longer rides along in the envelope, matching what the unmigrated rows and query routes have always done. Adds a compile-time tie between the shared table fixture and TableDefinition. tsconfig excludes test files, so an assertion placed in one is never checked; a new required field was a production type error and a silent no-op across every route test. Corrects the getRowSummaryById TSDoc, which claimed the Copilot row tool never put executions on the wire. It did, and that narrowing is a deliberate wire change rather than a pure saving. * test(table): key the attribution audit on the write, not on the id reader The allowlist matched the `readClientId` call site. That reader is a general-purpose helper any surface may call for unrelated reasons, so an innocent caller elsewhere in the app would be classified as naming a tab, and aliasing or wrapping the reader would slip past it. Match the two forms that actually attribute a write instead -- setting `actorClientId`, or passing a second argument to the signal -- which hold however the id was obtained and say nothing about unrelated readers.
This commit is contained in:
@@ -8,12 +8,19 @@ import {
|
||||
PRIVATE_SECRET_PROVENANCE_BUNDLE_V1,
|
||||
PRIVATE_SECRET_PROVENANCE_FIELD,
|
||||
PRIVATE_SECRET_PROVENANCE_HEADER,
|
||||
PRIVATE_TOOL_METADATA_REQUEST_HEADER,
|
||||
PRIVATE_TOOL_METADATA_RESPONSE_HEADER,
|
||||
RESOLVED_SECRET_PROVENANCE_METADATA_V1,
|
||||
} from '@/lib/execution/private-tool-metadata'
|
||||
import { TableRowProvenanceError } from '@/lib/table/application/row-secret-provenance'
|
||||
import { rowDataNameToId } from '@/lib/table/column-keys'
|
||||
import { tableRowSecretProvenanceSelectionKey } from '@/lib/table/secret-provenance-selection'
|
||||
import type { RowData } from '@/lib/table/types'
|
||||
import {
|
||||
createTableWriteProvenanceTargets,
|
||||
finalizeTableRowsProvenance,
|
||||
negotiateTableRowsProvenance,
|
||||
readTableRowProvenanceEnvelope,
|
||||
resolveTableWriteSecretProvenance,
|
||||
} from '@/app/api/table/row-secret-provenance'
|
||||
|
||||
@@ -258,3 +265,77 @@ describe('resolveTableWriteSecretProvenance', () => {
|
||||
expect(result.success).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* The transport half of the envelope, used by the migrated single-row routes.
|
||||
*
|
||||
* These are the only cover these helpers have: the route tests assert `mapInput`
|
||||
* and `present`, so each helper could be replaced by a constant without a route
|
||||
* test noticing — and a constant `readTableRowProvenanceEnvelope` would silently
|
||||
* downgrade every executor write from a stamped bundle to untracked.
|
||||
*/
|
||||
describe('readTableRowProvenanceEnvelope', () => {
|
||||
it('reports no envelope when the caller sent none', () => {
|
||||
const request = createMockRequest('PATCH', { data: {} })
|
||||
|
||||
expect(readTableRowProvenanceEnvelope(request, { data: {} })).toEqual({ kind: 'none' })
|
||||
})
|
||||
|
||||
it('hands the verified bundle over unresolved', () => {
|
||||
const { request, payload } = bundleRequest([tableRowSecretProvenanceSelectionKey(0, 'email')])
|
||||
|
||||
const envelope = readTableRowProvenanceEnvelope(request, payload)
|
||||
|
||||
expect(envelope.kind).toBe('bundle')
|
||||
expect(envelope).toEqual({ kind: 'bundle', value: payload[PRIVATE_SECRET_PROVENANCE_FIELD] })
|
||||
})
|
||||
|
||||
it('rejects a declared bundle whose payload field is missing', () => {
|
||||
const request = createMockRequest(
|
||||
'PATCH',
|
||||
{ data: {} },
|
||||
{ [PRIVATE_SECRET_PROVENANCE_HEADER]: PRIVATE_SECRET_PROVENANCE_BUNDLE_V1 }
|
||||
)
|
||||
|
||||
expect(() => readTableRowProvenanceEnvelope(request, { data: {} })).toThrow(
|
||||
TableRowProvenanceError
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('negotiateTableRowsProvenance', () => {
|
||||
it('is not requested without the capability header', () => {
|
||||
expect(negotiateTableRowsProvenance(createMockRequest('GET', undefined), true)).toBe(false)
|
||||
})
|
||||
|
||||
it('is accepted for an internal caller that asked for it', () => {
|
||||
const request = createMockRequest('GET', undefined, {
|
||||
[PRIVATE_TOOL_METADATA_REQUEST_HEADER]: RESOLVED_SECRET_PROVENANCE_METADATA_V1,
|
||||
})
|
||||
|
||||
expect(negotiateTableRowsProvenance(request, true)).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects a session caller that asks for the internal capability', () => {
|
||||
const request = createMockRequest('GET', undefined, {
|
||||
[PRIVATE_TOOL_METADATA_REQUEST_HEADER]: RESOLVED_SECRET_PROVENANCE_METADATA_V1,
|
||||
})
|
||||
|
||||
expect(() => negotiateTableRowsProvenance(request, false)).toThrow(TableRowProvenanceError)
|
||||
})
|
||||
})
|
||||
|
||||
describe('finalizeTableRowsProvenance', () => {
|
||||
it('adds nothing when the use case loaded no provenance', () => {
|
||||
expect(finalizeTableRowsProvenance(undefined)).toEqual({})
|
||||
})
|
||||
|
||||
it('adds the sibling body field and the capability header when it did', () => {
|
||||
const finalized = finalizeTableRowsProvenance({ rows: [] })
|
||||
|
||||
expect(finalized.bodyFields).toBeDefined()
|
||||
expect(new Headers(finalized.headers).get(PRIVATE_TOOL_METADATA_RESPONSE_HEADER)).toBe(
|
||||
RESOLVED_SECRET_PROVENANCE_METADATA_V1
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -14,6 +14,8 @@ const {
|
||||
mockDownloadWorkspaceFile,
|
||||
mockGetTableById,
|
||||
mockBatchInsertRows,
|
||||
mockInsertRow,
|
||||
mockUpdateRow,
|
||||
mockReplaceTableRows,
|
||||
mockAddWorkflowGroup,
|
||||
mockCreateTable,
|
||||
@@ -42,6 +44,8 @@ const {
|
||||
mockDownloadWorkspaceFile: vi.fn(),
|
||||
mockGetTableById: vi.fn(),
|
||||
mockBatchInsertRows: vi.fn(),
|
||||
mockInsertRow: vi.fn(),
|
||||
mockUpdateRow: vi.fn(),
|
||||
mockReplaceTableRows: vi.fn(),
|
||||
mockAddWorkflowGroup: vi.fn(),
|
||||
mockCreateTable: vi.fn(),
|
||||
@@ -208,10 +212,10 @@ vi.mock('@/lib/table/rows/service', () => ({
|
||||
deleteRowsByFilter: mockDeleteRowsByFilter,
|
||||
deleteRowsByIds: vi.fn(),
|
||||
getRowById: vi.fn(),
|
||||
insertRow: vi.fn(),
|
||||
insertRow: mockInsertRow,
|
||||
queryRows: mockQueryRows,
|
||||
replaceTableRows: mockReplaceTableRows,
|
||||
updateRow: vi.fn(),
|
||||
updateRow: mockUpdateRow,
|
||||
updateRowsByFilter: mockUpdateRowsByFilter,
|
||||
}))
|
||||
|
||||
@@ -1669,3 +1673,73 @@ describe('userTableServerTool.delete bounds', () => {
|
||||
expect(mockGetTableById).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* Copilot is the one row-write surface whose column keys come from a model rather
|
||||
* than from the schema, so `dataKeying: 'names'` is what stands between an
|
||||
* LLM-authored key and the storage column it means.
|
||||
*
|
||||
* These pin the translated outcome rather than the literal: the shared `buildTable`
|
||||
* fixture uses legacy columns with no `id`, where name-to-id mapping is the identity
|
||||
* and flipping the keying is unobservable. Columns whose `id` differs from `name` are
|
||||
* what make the wrong keying fail — under `'ids'` the lax write path stores the
|
||||
* model's key verbatim and reports success, corrupting the row silently.
|
||||
*/
|
||||
describe('userTableServerTool row writes key model-supplied columns by name', () => {
|
||||
const KEYED_TABLE = buildTable({
|
||||
schema: {
|
||||
columns: [
|
||||
{ id: 'col_name', name: 'name', type: 'string', required: true },
|
||||
{ id: 'col_age', name: 'age', type: 'number' },
|
||||
],
|
||||
},
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockGetTableById.mockResolvedValue(KEYED_TABLE)
|
||||
})
|
||||
|
||||
it('translates an inserted row to storage column ids', async () => {
|
||||
mockInsertRow.mockResolvedValue({
|
||||
id: 'row-1',
|
||||
data: { col_name: 'Ada', col_age: 36 },
|
||||
position: 0,
|
||||
createdAt: new Date('2024-01-01'),
|
||||
updatedAt: new Date('2024-01-01'),
|
||||
})
|
||||
|
||||
await userTableServerTool.execute(
|
||||
{
|
||||
operation: 'insert_row',
|
||||
args: { tableId: 'tbl_1', data: { name: 'Ada', age: 36 } },
|
||||
},
|
||||
buildToolContext()
|
||||
)
|
||||
|
||||
expect(mockInsertRow).toHaveBeenCalledTimes(1)
|
||||
expect(mockInsertRow.mock.calls[0][0].data).toEqual({ col_name: 'Ada', col_age: 36 })
|
||||
})
|
||||
|
||||
it('translates an updated row to storage column ids', async () => {
|
||||
mockUpdateRow.mockResolvedValue({
|
||||
id: 'row-1',
|
||||
data: { col_name: 'Grace' },
|
||||
position: 0,
|
||||
executions: {},
|
||||
createdAt: new Date('2024-01-01'),
|
||||
updatedAt: new Date('2024-01-01'),
|
||||
})
|
||||
|
||||
await userTableServerTool.execute(
|
||||
{
|
||||
operation: 'update_row',
|
||||
args: { tableId: 'tbl_1', rowId: 'row-1', data: { name: 'Grace' } },
|
||||
},
|
||||
buildToolContext()
|
||||
)
|
||||
|
||||
expect(mockUpdateRow).toHaveBeenCalledTimes(1)
|
||||
expect(mockUpdateRow.mock.calls[0][0].data).toEqual({ col_name: 'Grace' })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -8,18 +8,13 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { mocks } = vi.hoisted(() => ({
|
||||
mocks: { scopeCompatible: vi.fn(() => true), isBundle: vi.fn(() => true) },
|
||||
mocks: { scopeCompatible: vi.fn(() => true) },
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/execution/durable-secret-provenance', () => ({
|
||||
isPrivateSecretProvenanceScopeCompatible: mocks.scopeCompatible,
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/execution/model-input-provenance', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('@/lib/execution/model-input-provenance')>()
|
||||
return { ...actual, isPrivateSecretProvenanceBundleV1: mocks.isBundle }
|
||||
})
|
||||
|
||||
import {
|
||||
resolveRowWriteProvenance,
|
||||
TableRowProvenanceError,
|
||||
@@ -49,6 +44,31 @@ const EXECUTOR = {
|
||||
expiresAt: new Date('2026-01-02'),
|
||||
}
|
||||
|
||||
/**
|
||||
* A provenance shape the real `isResolvedSecretTraceProvenanceV1` accepts.
|
||||
*
|
||||
* These fixtures run against the genuine envelope guard rather than a stub, so a
|
||||
* bundle that could never survive the wire cannot pass here either.
|
||||
*/
|
||||
function traceProvenance(scope?: { userId: string; workspaceId: string }) {
|
||||
return { version: 1 as const, complete: true, entries: [], ...(scope ? { scope } : {}) }
|
||||
}
|
||||
|
||||
/** A bundle shape the real `isPrivateSecretProvenanceBundleV1` accepts. */
|
||||
function bundle(
|
||||
selections: Array<{ key: string; scope?: { userId: string; workspaceId: string } }>,
|
||||
complete = true
|
||||
) {
|
||||
return {
|
||||
version: 1 as const,
|
||||
complete,
|
||||
selections: selections.map((selection) => ({
|
||||
key: selection.key,
|
||||
provenance: traceProvenance(selection.scope),
|
||||
})),
|
||||
}
|
||||
}
|
||||
|
||||
function resolve(overrides: Partial<Parameters<typeof resolveRowWriteProvenance>[0]>) {
|
||||
return resolveRowWriteProvenance({
|
||||
envelope: { kind: 'none' },
|
||||
@@ -80,24 +100,40 @@ describe('row write provenance', () => {
|
||||
})
|
||||
|
||||
it('refuses a bundle from a session caller', () => {
|
||||
expect(() =>
|
||||
resolve({ envelope: { kind: 'bundle', value: { complete: true, selections: [] } } })
|
||||
).toThrow(TableRowProvenanceError)
|
||||
expect(() => resolve({ envelope: { kind: 'bundle', value: bundle([]) } })).toThrow(
|
||||
TableRowProvenanceError
|
||||
)
|
||||
})
|
||||
|
||||
it('refuses a bundle that is not a recognised envelope', () => {
|
||||
mocks.isBundle.mockReturnValueOnce(false)
|
||||
|
||||
expect(() =>
|
||||
resolve({ principal: EXECUTOR, envelope: { kind: 'bundle', value: { nope: true } } })
|
||||
).toThrow(TableRowProvenanceError)
|
||||
})
|
||||
|
||||
it('refuses a bundle whose selections carry an unrecognised provenance shape', () => {
|
||||
expect(() =>
|
||||
resolve({
|
||||
principal: EXECUTOR,
|
||||
envelope: {
|
||||
kind: 'bundle',
|
||||
value: {
|
||||
version: 1,
|
||||
complete: true,
|
||||
selections: [
|
||||
{ key: JSON.stringify([0, 'col_aaa']), provenance: { scope: { kind: 'workspace' } } },
|
||||
],
|
||||
},
|
||||
},
|
||||
})
|
||||
).toThrow(TableRowProvenanceError)
|
||||
})
|
||||
|
||||
it('refuses a complete bundle that does not account for every written cell', () => {
|
||||
expect(() =>
|
||||
resolve({
|
||||
principal: EXECUTOR,
|
||||
envelope: { kind: 'bundle', value: { complete: true, selections: [] } },
|
||||
envelope: { kind: 'bundle', value: bundle([]) },
|
||||
wireRows: [{ col_aaa: 'Ada', col_bbb: 36 }],
|
||||
storageRows: [{ col_aaa: 'Ada', col_bbb: 36 }],
|
||||
})
|
||||
@@ -112,12 +148,7 @@ describe('row write provenance', () => {
|
||||
principal: EXECUTOR,
|
||||
envelope: {
|
||||
kind: 'bundle',
|
||||
value: {
|
||||
complete: true,
|
||||
selections: [
|
||||
{ key: JSON.stringify([0, 'col_aaa']), provenance: { scope: { kind: 'workspace' } } },
|
||||
],
|
||||
},
|
||||
value: bundle([{ key: JSON.stringify([0, 'col_aaa']) }]),
|
||||
},
|
||||
})
|
||||
).toThrow(TableRowProvenanceError)
|
||||
@@ -126,7 +157,7 @@ describe('row write provenance', () => {
|
||||
it('marks an incomplete bundle unknown rather than certifying it', () => {
|
||||
const { stamps } = resolve({
|
||||
principal: EXECUTOR,
|
||||
envelope: { kind: 'bundle', value: { complete: false, selections: [] } },
|
||||
envelope: { kind: 'bundle', value: bundle([], false) },
|
||||
})
|
||||
|
||||
expect(stamps).toEqual([{ complete: false, columns: {} }])
|
||||
@@ -138,21 +169,10 @@ describe('row write provenance', () => {
|
||||
keying: 'names',
|
||||
wireRows: [{ Name: 'Ada' }],
|
||||
storageRows: [{ col_aaa: 'Ada' }],
|
||||
envelope: {
|
||||
kind: 'bundle',
|
||||
value: {
|
||||
complete: true,
|
||||
selections: [
|
||||
{ key: JSON.stringify([0, 'Name']), provenance: { scope: { kind: 'workspace' } } },
|
||||
],
|
||||
},
|
||||
},
|
||||
envelope: { kind: 'bundle', value: bundle([{ key: JSON.stringify([0, 'Name']) }]) },
|
||||
})
|
||||
|
||||
expect(stamps[0]).toEqual({
|
||||
complete: true,
|
||||
columns: { col_aaa: { scope: { kind: 'workspace' } } },
|
||||
})
|
||||
expect(stamps[0]).toEqual({ complete: true, columns: { col_aaa: traceProvenance() } })
|
||||
})
|
||||
|
||||
it('checks the scope against the acting principal, not a billing owner', () => {
|
||||
@@ -160,15 +180,17 @@ describe('row write provenance', () => {
|
||||
principal: EXECUTOR,
|
||||
envelope: {
|
||||
kind: 'bundle',
|
||||
value: {
|
||||
complete: true,
|
||||
selections: [{ key: JSON.stringify([0, 'col_aaa']), provenance: { scope: {} } }],
|
||||
},
|
||||
value: bundle([
|
||||
{
|
||||
key: JSON.stringify([0, 'col_aaa']),
|
||||
scope: { userId: 'billing-owner', workspaceId: 'workspace-1' },
|
||||
},
|
||||
]),
|
||||
},
|
||||
})
|
||||
|
||||
expect(mocks.scopeCompatible).toHaveBeenCalledWith(
|
||||
{},
|
||||
{ userId: 'billing-owner', workspaceId: 'workspace-1' },
|
||||
{ userId: 'user-1', workspaceId: 'workspace-1' }
|
||||
)
|
||||
})
|
||||
@@ -182,16 +204,10 @@ describe('row write provenance', () => {
|
||||
keying: 'ids',
|
||||
wireRows: [{ 'col-unknown': 'x' }],
|
||||
storageRows: [{ 'col-unknown': 'x' }],
|
||||
envelope: {
|
||||
kind: 'bundle',
|
||||
value: {
|
||||
complete: true,
|
||||
selections: [{ key: JSON.stringify([0, 'col-unknown']), provenance: { scope: {} } }],
|
||||
},
|
||||
},
|
||||
envelope: { kind: 'bundle', value: bundle([{ key: JSON.stringify([0, 'col-unknown']) }]) },
|
||||
})
|
||||
|
||||
expect(stamps[0]).toEqual({ complete: true, columns: { 'col-unknown': { scope: {} } } })
|
||||
expect(stamps[0]).toEqual({ complete: true, columns: { 'col-unknown': traceProvenance() } })
|
||||
})
|
||||
|
||||
it('records nothing for a key that names no column, since it is never stored', () => {
|
||||
@@ -200,15 +216,7 @@ describe('row write provenance', () => {
|
||||
keying: 'names',
|
||||
wireRows: [{ Nope: 'x' }],
|
||||
storageRows: [{}],
|
||||
envelope: {
|
||||
kind: 'bundle',
|
||||
value: {
|
||||
complete: true,
|
||||
selections: [
|
||||
{ key: JSON.stringify([0, 'Nope']), provenance: { scope: { kind: 'workspace' } } },
|
||||
],
|
||||
},
|
||||
},
|
||||
envelope: { kind: 'bundle', value: bundle([{ key: JSON.stringify([0, 'Nope']) }]) },
|
||||
})
|
||||
|
||||
expect(stamps[0]).toEqual({ complete: true, columns: {} })
|
||||
|
||||
@@ -727,10 +727,13 @@ describe('row query and upsert application semantics', () => {
|
||||
},
|
||||
})
|
||||
|
||||
expect(mockLoadSecretProvenance).toHaveBeenCalledWith([row], {
|
||||
userId: 'user-1',
|
||||
workspaceId: TABLE.workspaceId,
|
||||
})
|
||||
// Narrowed to the columns the row still holds: without `selectedValues` a
|
||||
// stale sidecar entry for a dropped column rides along in the envelope, and
|
||||
// the unmigrated `rows`/`query` routes have always narrowed here.
|
||||
expect(mockLoadSecretProvenance).toHaveBeenCalledWith(
|
||||
[{ id: row.id, updatedAt: row.updatedAt, selectedValues: row.data }],
|
||||
{ userId: 'user-1', workspaceId: TABLE.workspaceId }
|
||||
)
|
||||
expect(result.secretProvenance).toBe(provenance)
|
||||
})
|
||||
|
||||
|
||||
@@ -120,16 +120,22 @@ type TableRowsProvenance = Awaited<ReturnType<typeof loadTableRowSecretProvenanc
|
||||
async function loadAuthorizedRowsProvenance(
|
||||
principal: Parameters<typeof requirePrincipalSubjectUserId>[0],
|
||||
workspaceId: string,
|
||||
// The loader reads only id, updatedAt and data, so a row without its
|
||||
// executions sidecar is enough — see `TABLE_ROW_SIDECAR_SELECTION`.
|
||||
// The loader reads only id, updatedAt and the selected values, so a row
|
||||
// without its executions sidecar is enough — see `TABLE_ROW_SIDECAR_SELECTION`.
|
||||
rows: TableRowSummary[],
|
||||
include: boolean | undefined
|
||||
): Promise<TableRowsProvenance | undefined> {
|
||||
if (!include) return undefined
|
||||
return loadTableRowSecretProvenance(rows, {
|
||||
userId: requirePrincipalSubjectUserId(principal),
|
||||
workspaceId,
|
||||
})
|
||||
return loadTableRowSecretProvenance(
|
||||
// `selectedValues` narrows the sidecar to the columns the row still holds.
|
||||
// Without it a stale entry for a dropped column rides along in the envelope,
|
||||
// which is how the unmigrated `rows`/`query` routes have always behaved.
|
||||
rows.map((row) => ({ id: row.id, updatedAt: row.updatedAt, selectedValues: row.data })),
|
||||
{
|
||||
userId: requirePrincipalSubjectUserId(principal),
|
||||
workspaceId,
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
function requestId(input: TableScopedInput): string {
|
||||
|
||||
@@ -26,11 +26,28 @@ const ATTRIBUTED_CALL_SITES = [
|
||||
] as const
|
||||
|
||||
/** Surfaces that name the acting tab. See the note above — this is the real allowlist. */
|
||||
const ACTOR_SUPPLYING_SURFACES = ['app/api/table/[tableId]/rows/[rowId]/route.ts'] as const
|
||||
const ACTOR_SUPPLYING_SURFACES = [
|
||||
'app/api/table/[tableId]/rows/route.ts',
|
||||
'app/api/table/[tableId]/rows/[rowId]/route.ts',
|
||||
] as const
|
||||
|
||||
const APP_ROOT = join(import.meta.dirname, '../..')
|
||||
/** Declares the function; matching its own definition would say nothing about call sites. */
|
||||
const DECLARING_MODULE = 'lib/table/events.ts'
|
||||
/** Declares the input and forwards it to the signal; it names no tab of its own. */
|
||||
const FORWARDING_MODULE = 'lib/table/application/rows.ts'
|
||||
|
||||
/**
|
||||
* A file names a tab either by setting the input field or by passing a second
|
||||
* argument to the signal directly.
|
||||
*
|
||||
* Deliberately keyed on the attribution rather than on `readClientId`: that reader
|
||||
* is a general-purpose helper any surface may call for unrelated reasons, so
|
||||
* matching it classified innocent callers as suppliers and could be sidestepped by
|
||||
* aliasing or wrapping it. These two forms are what actually attribute a write, and
|
||||
* they hold however the id was obtained.
|
||||
*/
|
||||
const SUPPLIER_PATTERNS = [/actorClientId:/, /signalTableRowsChangedByActor\([^)]*,/] as const
|
||||
|
||||
async function* walk(dir: string): AsyncGenerator<string> {
|
||||
for (const entry of await readdir(dir, { withFileTypes: true })) {
|
||||
@@ -41,11 +58,14 @@ async function* walk(dir: string): AsyncGenerator<string> {
|
||||
}
|
||||
}
|
||||
|
||||
async function filesContaining(needle: string, skip: (relative: string) => boolean = () => false) {
|
||||
async function filesMatching(
|
||||
matches: (source: string) => boolean,
|
||||
skip: (relative: string) => boolean = () => false
|
||||
) {
|
||||
const found: string[] = []
|
||||
for await (const file of walk(APP_ROOT)) {
|
||||
const source = await readFile(file, 'utf8')
|
||||
if (!source.includes(needle)) continue
|
||||
if (!matches(source)) continue
|
||||
const relative = file.slice(APP_ROOT.length + 1)
|
||||
if (skip(relative)) continue
|
||||
found.push(relative)
|
||||
@@ -55,8 +75,8 @@ async function filesContaining(needle: string, skip: (relative: string) => boole
|
||||
|
||||
describe('signalTableRowsChangedByActor call sites', () => {
|
||||
it('is called only where the acting tab reconciles the write locally', async () => {
|
||||
const callers = await filesContaining(
|
||||
'signalTableRowsChangedByActor(',
|
||||
const callers = await filesMatching(
|
||||
(source) => source.includes('signalTableRowsChangedByActor('),
|
||||
(relative) => relative === DECLARING_MODULE
|
||||
)
|
||||
|
||||
@@ -64,12 +84,9 @@ describe('signalTableRowsChangedByActor call sites', () => {
|
||||
})
|
||||
|
||||
it('is given an actor only by surfaces whose client hook reconciles locally', async () => {
|
||||
const suppliers = await filesContaining(
|
||||
'actorClientId:',
|
||||
// These declare or forward the field rather than naming a tab.
|
||||
(relative) =>
|
||||
relative === 'lib/table/application/rows.ts' ||
|
||||
relative === 'lib/table/application/row-secret-provenance.ts'
|
||||
const suppliers = await filesMatching(
|
||||
(source) => SUPPLIER_PATTERNS.some((pattern) => pattern.test(source)),
|
||||
(relative) => relative === DECLARING_MODULE || relative === FORWARDING_MODULE
|
||||
)
|
||||
|
||||
expect(suppliers).toEqual([...ACTOR_SUPPLYING_SURFACES].sort())
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
import type { createTableDefinition } from '@sim/testing'
|
||||
import type { TableDefinition } from '@/lib/table/types'
|
||||
|
||||
/**
|
||||
* Compile-time tie between the shared table fixture and the domain type it stands in for.
|
||||
*
|
||||
* `createTableDefinition` returns a `TableDefinitionFixture` declared inside `@sim/testing`,
|
||||
* which cannot import from the apps workspace, so the two shapes are only structurally
|
||||
* related. The fixture then flows into untyped `vi.fn().mockResolvedValue(...)` calls, which
|
||||
* erase the type entirely — and `tsconfig.json` excludes test files, so no assertion placed
|
||||
* in one would ever be checked. Without this probe a new required field on
|
||||
* {@link TableDefinition} is a production type error and a silent no-op across every route
|
||||
* test, leaving them to exercise a table shape that no longer exists.
|
||||
*
|
||||
* Type-only by construction: `@sim/testing` is a devDependency, so this file must never emit
|
||||
* a runtime import.
|
||||
*/
|
||||
type AssertAssignableToTableDefinition<T extends TableDefinition> = T
|
||||
|
||||
export type TableFixtureMatchesDomainType = AssertAssignableToTableDefinition<
|
||||
ReturnType<typeof createTableDefinition>
|
||||
>
|
||||
@@ -1438,9 +1438,15 @@ function toRowSummary(row: Awaited<ReturnType<typeof selectRowRecord>>[number]):
|
||||
}
|
||||
|
||||
/**
|
||||
* One row without its executions sidecar, for the surfaces that never put
|
||||
* executions on the wire — the single-row read routes and the Copilot row tool.
|
||||
* Loading the sidecar for them is a query whose result is discarded.
|
||||
* One row without its executions sidecar, for the single-row read routes, which
|
||||
* project `id`/`data`/`position`/`createdAt`/`updatedAt` and never put executions
|
||||
* on the wire. Loading the sidecar for them is a query whose result is discarded.
|
||||
*
|
||||
* Not every `readTableRow` caller is such a surface: the Copilot `get_row` tool
|
||||
* spreads the row straight onto its result, so it used to hand the model an
|
||||
* executions map and no longer does. That narrowing is deliberate — the generated
|
||||
* tool contract never described the field, and the bulk `query_rows` path has
|
||||
* always returned an empty one — but it is a wire change, not a pure saving.
|
||||
*
|
||||
* Deliberately a separate function rather than a flag on {@link getRowById}: a
|
||||
* caller that forgets to pass the flag reads an empty sidecar and cannot tell
|
||||
|
||||
Reference in New Issue
Block a user