diff --git a/scripts/layering/session-state.test.ts b/scripts/layering/session-state.test.ts index e243df258e..c2c12bdb12 100644 --- a/scripts/layering/session-state.test.ts +++ b/scripts/layering/session-state.test.ts @@ -1,5 +1,6 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; +import { parseSync } from 'oxc-parser'; import { findSessionStateWrites, SESSION_STATE_FIELD_OWNERS, @@ -9,6 +10,7 @@ import { const OWNER = 'src/daemon/app-log-session-resource.ts'; const FIELDS = ['appLog', 'appLogFailure', 'lease', 'lastPerfProfile']; function scan(source: string, file = OWNER) { + assert.deepEqual(parseSync(file, source).errors, [], `fixture must parse as a module: ${file}`); return findSessionStateWrites(new Map([[file, source]]), FIELDS); } @@ -80,9 +82,9 @@ test('the historical app-log whole-record spread is refused and counted fairly', test('record copies through a read alias or captured ref remain visible', () => { for (const source of [ - 'const refreshed = params.sessionStore.get(address) ?? session; return { ...refreshed, lastPerfProfile: profile };', - 'const previous: SessionState = value; return { ...previous, lastPerfProfile: profile };', - 'return { ...ref.session, lastPerfProfile: profile };', + 'function copy(session: SessionState) { const refreshed = params.sessionStore.get(address) ?? session; return { ...refreshed, lastPerfProfile: profile }; }', + 'function copy(value: SessionState) { const previous: SessionState = value; return { ...previous, lastPerfProfile: profile }; }', + 'function copy(ref: SessionRef) { return { ...ref.session, lastPerfProfile: profile }; }', ]) assert.deepEqual( scan(source).map((w) => w.field), @@ -114,8 +116,8 @@ test('plain store and record aliases retain their owning identity', () => { ['appLogFailure'], ); for (const source of [ - 'const current = ref.session; return { ...current };', - 'const current = session; return { ...current };', + 'function copy(ref: SessionRef) { const current = ref.session; return { ...current }; }', + 'function copy(session: SessionState) { const current = session; return { ...current }; }', ]) { assert.deepEqual( scan(source).map((w) => w.field), @@ -125,12 +127,121 @@ test('plain store and record aliases retain their owning identity', () => { } }); +test('direct field writes through a tracked SessionRef session are visible', () => { + for (const source of [ + 'function mutate(ref: SessionRef) { ref.session.appLogFailure = error; }', + 'function mutate(store: SessionStore) { const ref = store.lookup(address); ref.session.appLogFailure = error; }', + 'function mutate(ref: SessionRef) { ref["session"].appLogFailure = error; }', + ]) + assert.deepEqual( + scan(source).map((write) => write.field), + ['appLogFailure'], + source, + ); + assert.deepEqual( + scan( + 'function mutate(ref: SessionRef) { const { session: current } = ref; current.appLogFailure = error; }', + ).map((write) => write.field), + ['appLogFailure'], + ); + assert.deepEqual( + scan( + 'function mutate(ref: SessionRef) { const current = ref?.session; current.appLogFailure = error; }', + ).map((write) => write.field), + ['appLogFailure'], + ); +}); + +test('unrelated session properties are not SessionState records', () => { + for (const source of [ + 'function copy(request: { session: string }) { return { ...request.session }; }', + 'function mutate(request: { session: string }) { const { session: current } = request; current.appLogFailure = error; }', + 'function copy(ref: SessionRef, session: string) { return { ...ref[session] }; }', + ]) + assert.deepEqual(scan(source), [], source); +}); + +test('optional-chain record and store reads remain visible through aliases', () => { + for (const source of [ + 'function copy(ref: SessionRef) { const current = ref?.session; return { ...current }; }', + 'function copy(ref: SessionRef) { return { ...ref?.session }; }', + 'function copy(ref: SessionRef) { return { ...ref["session"] }; }', + 'function copy(sessionStore: SessionStore, ref: SessionRef) { const current = sessionStore?.get(ref); return { ...current }; }', + ]) { + assert.deepEqual( + scan(source).map((write) => write.field), + ['[whole-record-spread]'], + source, + ); + } +}); + +test('direct clone APIs and full object-rest patterns cannot copy a session record', () => { + for (const source of [ + 'function copy(session: SessionState) { return Object.assign({}, session); }', + 'function copy(session: SessionState) { return structuredClone(session); }', + 'function copy(session: SessionState) { const { ...copy } = session; }', + ]) { + assert.deepEqual( + scan(source).map((write) => write.field), + ['[whole-record-spread]'], + source, + ); + } + assert.deepEqual( + scan( + 'function copy(session: SessionState) { const { lease, ...rest } = session; rest.appLogFailure = error; }', + ).map((write) => write.field), + ['[whole-record-spread]', 'appLogFailure'], + ); + assert.deepEqual( + scan( + 'function copy(params: object, key: string) { const { [key]: current } = params; current.appLogFailure = error; }', + ), + [], + ); + assert.deepEqual( + scan( + 'function copy(params: object, session: string) { const { [session]: current } = params; current.appLogFailure = error; }', + ), + [], + ); +}); + +test('nested callback returns do not make a direct patch return ambiguous', () => { + for (const source of [ + 'store.update(ref, (current) => { const inspect = () => { if (current) return; }; return { lease: current.lease }; });', + 'store.update(ref, (current) => { items.forEach((item) => { if (item) return; }); return { lease: { ...current.lease } }; });', + ]) { + assert.deepEqual( + scan(source).map((write) => write.field), + ['lease'], + source, + ); + } +}); + +test('rest copies retain identity from an inline patch callback current record', () => { + for (const record of ['current', 'previous']) { + const source = `store.update(ref, (current) => { + const previous = current; + const { lease, ...copy } = ${record}; + return { appLogFailure: copy.appLogFailure }; + });`; + assert.deepEqual( + scan(source).map((write) => write.field), + ['[whole-record-spread]', 'appLogFailure'], + source, + ); + } +}); + test('destructuring preserves store and record aliases', () => { for (const pattern of ['{ session: current }', '{ session: current = fallback }']) { assert.deepEqual( - scan(`const ${pattern} = ref; return { ...current, appLogFailure: error };`).map( - (w) => w.field, - ), + scan( + `function copy(ref: SessionRef) { const ${pattern} = ref; return { ...current, appLogFailure: error }; }`, + ).map((w) => w.field), ['[whole-record-spread]', 'appLogFailure'], ); } @@ -148,6 +259,83 @@ test('destructuring preserves store and record aliases', () => { ); }); +test('computed destructuring from a typed SessionRef cannot hide its session record', () => { + for (const source of [ + 'function copy(ref: SessionRef, key: string) { const { [key]: current } = ref; return { ...current }; }', + 'function copy({ [key]: current }: SessionRef) { return { ...current }; }', + ]) { + assert.deepEqual( + scan(source).map((write) => write.field), + ['[whole-record-spread]'], + source, + ); + } + assert.deepEqual( + scan( + 'function copy(params: object, key: string) { const { [key]: current } = params; return { ...current }; }', + ), + [], + ); +}); + +test('literal computed SessionRef fields survive real store and typed parameter sources', () => { + const key = 'const key = "session"; '; + const fromLookup = + 'const ref = store.lookup(address); const { [key]: current } = ref; current.appLogFailure = error;'; + assert.deepEqual( + scan(key + fromLookup).map((write) => write.field), + ['appLogFailure'], + ); + assert.deepEqual( + scan( + 'function mutate(params: { ref: SessionRef }) { const key = "session"; const { [key]: current } = params.ref; current.appLogFailure = error; }', + ).map((write) => write.field), + ['appLogFailure'], + ); +}); + +test('known SessionRef-returning store methods seed computed destructuring', () => { + for (const read of [ + 'store.lookup(address)', + 'store.publish(address, session)', + 'store.findByDevice(deviceId)', + 'store.refresh(ref)', + ]) { + assert.deepEqual( + scan( + `const result = ${read}; const key = "session"; const { [key]: current } = result; current.lease = lease;`, + ).map((write) => write.field), + ['lease'], + read, + ); + } +}); + +test('static destructuring carries typed ref properties into computed record reads', () => { + for (const pattern of ['{ ref }', "{ ['ref']: ref }", '{ ref: ref = fallback }']) { + const source = `function mutate(params: { ref: SessionRef }) { + const ${pattern} = params; + const key = 'session'; + const { [key]: current } = ref; + current.appLogFailure = error; + }`; + assert.deepEqual( + scan(source).map((write) => write.field), + ['appLogFailure'], + source, + ); + } + assert.deepEqual( + scan(`function mutate(params: { ref: object }) { + const { ref } = params; + const key = 'session'; + const { [key]: current } = ref; + current.appLogFailure = error; + }`), + [], + ); +}); + test('alias declarations and patch parameters are collected before checking writes', () => { assert.deepEqual( scan( @@ -213,7 +401,10 @@ test('the owning store can merge records, while unrelated updates and platform s ); assert.deepEqual(scan('coordinator.update((session) => ({})); hash.update(data);'), []); assert.deepEqual( - scan('return { ...session, appLogFailure: error };', 'packages/platform-apple/src/session.ts'), + scan( + 'function copy(session: SessionState) { return { ...session, appLogFailure: error }; }', + 'packages/platform-apple/src/session.ts', + ), [], ); }); diff --git a/scripts/layering/session-state.ts b/scripts/layering/session-state.ts index 674026f507..1388abc843 100644 --- a/scripts/layering/session-state.ts +++ b/scripts/layering/session-state.ts @@ -189,7 +189,7 @@ function isSessionBinding(name: string): boolean { /** A member expression being assigned to, or updated with `++`/`--`. */ type WriteTarget = { - object: string | undefined; + object: unknown; field: string | undefined; computed: boolean; offset: number; @@ -197,19 +197,18 @@ type WriteTarget = { function writeTarget(node: Record): WriteTarget | null { const type = node['type']; - const member = + const candidate = type === 'AssignmentExpression' ? (node['left'] as Record | undefined) : type === 'UpdateExpression' ? (node['argument'] as Record | undefined) : undefined; - if (!member || member['type'] !== 'MemberExpression') return null; + const member = unwrapExpression(candidate); + if (member?.['type'] !== 'MemberExpression') return null; const object = member['object'] as Record | undefined; const property = member['property'] as Record | undefined; return { - // Only a direct `.field` write is a session write; `a.b.c = …` writes into a - // sub-object and its `object` is a MemberExpression, so it has no identifier name here. - object: object?.['type'] === 'Identifier' ? (object['name'] as string) : undefined, + object, field: property?.['type'] === 'Identifier' ? (property['name'] as string) : undefined, computed: member['computed'] === true, offset: typeof member['start'] === 'number' ? member['start'] : 0, @@ -241,6 +240,7 @@ function unwrapExpression(value: unknown): AstNode | undefined { 'TSSatisfiesExpression', 'TSNonNullExpression', 'ParenthesizedExpression', + 'ChainExpression', ].includes(String(node.type)) ) { node = astNode(node.expression); @@ -253,11 +253,20 @@ function isSessionStoreReceiver(value: unknown, storeBindings: ReadonlySet): boolean { +function isSessionRecord( + value: unknown, + sessionBindings: ReadonlySet, + sessionRefBindings: ReadonlySet, + sessionRefPaths: ReadonlySet, + storeBindings: ReadonlySet, +): boolean { const node = unwrapExpression(value); return node?.type === 'Identifier' ? sessionBindings.has(String(node.name)) - : node?.type === 'MemberExpression' && memberName(node) === 'session'; + : node?.type === 'MemberExpression' && + memberName(node) === 'session' && + (node.computed !== true || astNode(node.property)?.type === 'Literal') && + isSessionRefValue(node.object, sessionRefBindings, sessionRefPaths, storeBindings); } function isSessionRead(value: unknown, storeBindings: ReadonlySet): boolean { @@ -273,11 +282,109 @@ function isSessionRead(value: unknown, storeBindings: ReadonlySet): bool ); } +function isSessionRecordOrRead( + value: unknown, + sessionBindings: ReadonlySet, + sessionRefBindings: ReadonlySet, + sessionRefPaths: ReadonlySet, + storeBindings: ReadonlySet, +): boolean { + return ( + isSessionRecord(value, sessionBindings, sessionRefBindings, sessionRefPaths, storeBindings) || + isSessionRead(value, storeBindings) + ); +} + +function isSessionRefValue( + value: unknown, + sessionRefBindings: ReadonlySet, + sessionRefPaths: ReadonlySet, + storeBindings: ReadonlySet, +): boolean { + const node = unwrapExpression(value); + if (!node) return false; + if (node.type === 'LogicalExpression') + return ( + isSessionRefValue(node.left, sessionRefBindings, sessionRefPaths, storeBindings) || + isSessionRefValue(node.right, sessionRefBindings, sessionRefPaths, storeBindings) + ); + if (node.type === 'Identifier' && sessionRefBindings.has(String(node.name))) return true; + if (node.type === 'MemberExpression') { + const path = memberPath(node); + if (path && sessionRefPaths.has(path.join('\0'))) return true; + } + const callee = astNode(node.callee); + return ( + node.type === 'CallExpression' && + callee?.type === 'MemberExpression' && + ['lookup', 'publish', 'findByDevice', 'refresh'].includes(memberName(callee) ?? '') && + isSessionStoreReceiver(callee.object, storeBindings) + ); +} + function hasNamedType(node: AstNode, name: string): boolean { const annotation = astNode(astNode(node.typeAnnotation)?.typeAnnotation); return annotation?.type === 'TSTypeReference' && propertyName(annotation.typeName) === name; } +function sessionRefPropertyPaths(node: AstNode): string[][] { + const paths: string[][] = []; + const collect = (type: AstNode | undefined, prefix: readonly string[]): void => { + if (type?.type !== 'TSTypeLiteral') return; + for (const member of type.members as AstNode[]) { + if (member.type !== 'TSPropertySignature') continue; + const field = propertyName(member.key); + if (!field) continue; + if (hasNamedType(member, 'SessionRef')) paths.push([...prefix, field]); + else collect(astNode(astNode(member.typeAnnotation)?.typeAnnotation), [...prefix, field]); + } + }; + collect(astNode(astNode(node.typeAnnotation)?.typeAnnotation), []); + return paths; +} + +function hasObjectRestPattern(pattern: AstNode): boolean { + const properties = pattern.properties as AstNode[] | undefined; + return properties?.some((property) => property.type === 'RestElement') === true; +} + +function collectPatchReturns(value: unknown, returns: AstNode[]): void { + if (Array.isArray(value)) { + for (const child of value) collectPatchReturns(child, returns); + return; + } + const node = astNode(value); + if (!node) return; + if (node.type === 'ReturnStatement') { + returns.push(node); + return; + } + if ( + ['FunctionDeclaration', 'FunctionExpression', 'ArrowFunctionExpression'].includes( + String(node.type), + ) + ) + return; + for (const child of Object.values(node)) collectPatchReturns(child, returns); +} + +function bindSessionRefProperties(pattern: AstNode, sessionBindings: Set): void { + for (const property of pattern.properties as AstNode[]) { + if (property.type !== 'Property') continue; + const key = astNode(property.key); + const field = propertyName(property.key); + if ( + property.computed === true + ? key?.type === 'Literal' && field !== 'session' + : field !== 'session' + ) + continue; + const value = astNode(property.value); + const binding = value?.type === 'AssignmentPattern' ? astNode(value.left) : value; + if (binding?.type === 'Identifier') sessionBindings.add(String(binding.name)); + } +} + function patchObjects(value: unknown): AstNode[] | undefined { const patch = unwrapExpression(value); if (!patch) return undefined; @@ -291,9 +398,7 @@ function patchObjects(value: unknown): AstNode[] | undefined { if (body?.type === 'ObjectExpression') return [body]; if (body?.type !== 'BlockStatement') return undefined; const returns: AstNode[] = []; - visitAst(body, (node) => { - if (node.type === 'ReturnStatement') returns.push(node); - }); + collectPatchReturns(body, returns); if (returns.length !== 1 || !(body.body as AstNode[]).includes(returns[0]!)) return undefined; const result = unwrapExpression(returns[0]!.argument); return result?.type === 'ObjectExpression' ? [result] : undefined; @@ -318,8 +423,12 @@ export function findSessionStateWrites( if (path.posix.basename(file) === 'session-store.ts') continue; const program = parseSync(file, source).program; const sessionBindings = new Set(); + const sessionRefBindings = new Set(); + const sessionRefPaths = new Set(); const storeBindings = new Set(['store', 'sessionStore']); const aliases: Array = []; + const objectDestructurings: Array = []; + const recordRestPatterns = new Map(); const patches = new Set(); const report = (node: AstNode, field: string): void => { writes.push({ file, line: lineOf(source, Number(node.start ?? 0)), field }); @@ -328,20 +437,31 @@ export function findSessionStateWrites( if (node.type === 'Identifier') { if (isSessionBinding(String(node.name)) || hasNamedType(node, 'SessionState')) sessionBindings.add(String(node.name)); + if (hasNamedType(node, 'SessionRef')) sessionRefBindings.add(String(node.name)); if (hasNamedType(node, 'SessionStore')) storeBindings.add(String(node.name)); + for (const path of sessionRefPropertyPaths(node)) + sessionRefPaths.add([String(node.name), ...path].join('\0')); } if (node.type === 'VariableDeclarator') { const id = astNode(node.id); - if (id?.type === 'Identifier') aliases.push([String(id.name), node.init]); + if (id?.type === 'Identifier') { + aliases.push([String(id.name), node.init]); + } + if (id?.type === 'ObjectPattern') { + objectDestructurings.push([id, node.init]); + if (hasObjectRestPattern(id)) recordRestPatterns.set(id, node.init); + } } if (node.type === 'ObjectPattern') { + if (hasNamedType(node, 'SessionRef')) bindSessionRefProperties(node, sessionBindings); + if (hasObjectRestPattern(node) && hasNamedType(node, 'SessionState')) + recordRestPatterns.set(node, undefined); for (const property of node.properties as AstNode[]) { if (property.type !== 'Property') continue; - const field = propertyName(property.key); + const field = property.computed === true ? undefined : propertyName(property.key); const value = astNode(property.value); const binding = value?.type === 'AssignmentPattern' ? astNode(value.left) : value; if (binding?.type !== 'Identifier') continue; - if (field === 'session') sessionBindings.add(String(binding.name)); if (field === 'store' || field === 'sessionStore') storeBindings.add(String(binding.name)); } @@ -358,11 +478,53 @@ export function findSessionStateWrites( } if ( !sessionBindings.has(name) && - (isSessionRecord(value, sessionBindings) || isSessionRead(value, storeBindings)) + (isSessionRecord( + value, + sessionBindings, + sessionRefBindings, + sessionRefPaths, + storeBindings, + ) || + isSessionRead(value, storeBindings)) ) { sessionBindings.add(name); added = true; } + if ( + !sessionRefBindings.has(name) && + isSessionRefValue(value, sessionRefBindings, sessionRefPaths, storeBindings) + ) { + sessionRefBindings.add(name); + added = true; + } + } + for (const [pattern, value] of objectDestructurings) { + if (isSessionRefValue(value, sessionRefBindings, sessionRefPaths, storeBindings)) { + const bindingCount = sessionBindings.size; + bindSessionRefProperties(pattern, sessionBindings); + if (sessionBindings.size !== bindingCount) added = true; + } + const sourcePath = memberPath(unwrapExpression(value)); + if (!sourcePath) continue; + for (const property of pattern.properties as AstNode[]) { + if ( + property.type !== 'Property' || + (property.computed === true && astNode(property.key)?.type !== 'Literal') + ) + continue; + const field = propertyName(property.key); + const value = astNode(property.value); + const binding = value?.type === 'AssignmentPattern' ? astNode(value.left) : value; + if ( + field && + binding?.type === 'Identifier' && + !sessionRefBindings.has(String(binding.name)) && + sessionRefPaths.has([...sourcePath, field].join('\0')) + ) { + sessionRefBindings.add(String(binding.name)); + added = true; + } + } } } while (added); }; @@ -412,6 +574,25 @@ export function findSessionStateWrites( } }); inheritAliases(); + for (const [pattern, source] of recordRestPatterns) { + if ( + hasNamedType(pattern, 'SessionState') || + isSessionRecordOrRead( + source, + sessionBindings, + sessionRefBindings, + sessionRefPaths, + storeBindings, + ) + ) { + report(pattern, '[whole-record-spread]'); + for (const property of pattern.properties as AstNode[]) { + const binding = property.type === 'RestElement' ? astNode(property.argument) : undefined; + if (binding?.type === 'Identifier') sessionBindings.add(String(binding.name)); + } + } + } + inheritAliases(); const walk = (value: unknown, ancestors: readonly AstNode[]): void => { if (Array.isArray(value)) { for (const child of value) walk(child, ancestors); @@ -420,16 +601,54 @@ export function findSessionStateWrites( const node = astNode(value); if (!node) return; const target = writeTarget(node); - if (target?.object !== undefined && sessionBindings.has(target.object)) { + if ( + target && + isSessionRecord( + target.object, + sessionBindings, + sessionRefBindings, + sessionRefPaths, + storeBindings, + ) + ) { if (target.computed) report(node, '[computed]'); else if (target.field !== undefined && declared.has(target.field)) report(node, target.field); } + if (node.type === 'CallExpression') { + const callee = unwrapExpression(node.callee); + const args = node.arguments as unknown[] | undefined; + const objectAssign = + callee?.type === 'MemberExpression' && + memberName(callee) === 'assign' && + astNode(callee.object)?.type === 'Identifier' && + astNode(callee.object)?.name === 'Object'; + const structuredClone = callee?.type === 'Identifier' && callee.name === 'structuredClone'; + if ( + (objectAssign || structuredClone) && + args?.some((argument) => + isSessionRecordOrRead( + argument, + sessionBindings, + sessionRefBindings, + sessionRefPaths, + storeBindings, + ), + ) + ) + report(node, '[whole-record-spread]'); + } if (node.type === 'ObjectExpression' && !patches.has(node)) { const properties = node.properties as AstNode[]; const copiesRecord = properties.some((property) => { if (property.type !== 'SpreadElement') return false; - return isSessionRecord(property.argument, sessionBindings); + return isSessionRecordOrRead( + property.argument, + sessionBindings, + sessionRefBindings, + sessionRefPaths, + storeBindings, + ); }); if (copiesRecord) { const enclosingFunction = [...ancestors]