From f0965e92b51c2f8f79c8a5be8f6165eb8616d9de Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sat, 12 Sep 2026 01:05:16 -0400 Subject: [PATCH] refactor(mobile): simplify RPC descriptors and fence the contract module --- .../rpc-operation-cast-fence.test.ts | 7 +-- .../transport/rpc-operation-compile-fence.ts | 41 ++-------------- .../src/transport/rpc-operation-contract.ts | 8 +--- .../rpc-operation-result-reader.test.ts | 2 - .../transport/rpc-operation-test-families.ts | 20 ++------ mobile/src/transport/rpc-operation.test.ts | 47 ------------------- mobile/src/transport/rpc-operation.ts | 2 - 7 files changed, 12 insertions(+), 115 deletions(-) diff --git a/mobile/src/transport/rpc-operation-cast-fence.test.ts b/mobile/src/transport/rpc-operation-cast-fence.test.ts index 67f99633201..83d76eb87ab 100644 --- a/mobile/src/transport/rpc-operation-cast-fence.test.ts +++ b/mobile/src/transport/rpc-operation-cast-fence.test.ts @@ -13,8 +13,8 @@ import { describe, expect, it } from 'vitest' * drift the contract removed — and it buys it silently, since the code still compiles and the * types still read as validated. * - * The fenced region is computed, not listed: a non-test file is in it if it imports the - * operation API, the operation contract or the result-reader factory, or if it re-exports a + * The fenced region includes the operation API, contract and result-reader factory, plus + * non-test files importing them and files that re-export a * file that is (transitively). Step 4's operation modules therefore land inside the fence the * moment they are written, with nothing to remember. * @@ -162,7 +162,7 @@ function moduleKey(path: string): string { return path.replace(/\.[jt]sx?$/, '') } -const region = new Set() +const region = new Set(scanned.filter((path) => REGION_SEEDS.has(moduleKey(path)))) for (const [path, { imports }] of edges) { if (imports.some((target) => REGION_SEEDS.has(target))) { region.add(path) @@ -209,6 +209,7 @@ describe('RPC operation cast fence', () => { it('puts every operation module in the fenced region', () => { for (const file of [ 'src/transport/rpc-operation.ts', + 'src/transport/rpc-operation-contract.ts', 'src/transport/rpc-operation-test-families.ts', 'src/transport/rpc-operation-compile-fence.ts', 'src/transport/rpc-operation-result-reader.ts', diff --git a/mobile/src/transport/rpc-operation-compile-fence.ts b/mobile/src/transport/rpc-operation-compile-fence.ts index 54344d7c7e2..3188774743c 100644 --- a/mobile/src/transport/rpc-operation-compile-fence.ts +++ b/mobile/src/transport/rpc-operation-compile-fence.ts @@ -22,27 +22,22 @@ import type { // acceptance policy, the interpretation barrier, or the send-side params for itself. Every // expect-error directive below is that claim as an assertion — tsc fails on a directive that // stops catching an error, so `pnpm --dir mobile typecheck` is the gate. Nothing here runs and -// no app code imports it. The FENCE markers are pinned by rpc-operation.test.ts. +// no app code imports it. declare const client: RpcClient -// FENCE: variant-readers-cannot-be-empty // @ts-expect-error a variant reader combinator must have at least one reader const _fenceEmptyVariantReaders = rpcResultVariants([]) -// FENCE: probe-cannot-take-a-reader export const fenceProbeWithReader: CapabilityProbeRpcDefinition<'worktree.ps', 'on-settle'> = { name: 'fence.probeWithReader', method: 'worktree.ps', acceptance: 'method-not-found-refusal', barrier: 'on-settle', - consumes: [], - schedules: [], // @ts-expect-error a refusal-code probe reads no payload, so it cannot carry a reader read: workspaceRowsReader } -// FENCE: decoding-policy-needs-a-reader // @ts-expect-error 'require-result-or-throw' has no value to return without a reader export const fenceDecodingWithoutReader: RequireResultRpcDefinition< 'worktree.ps', @@ -53,12 +48,9 @@ export const fenceDecodingWithoutReader: RequireResultRpcDefinition< name: 'fence.decodingWithoutReader', method: 'worktree.ps', acceptance: 'require-result-or-throw', - barrier: 'on-settle', - consumes: [], - schedules: [] + barrier: 'on-settle' } -// FENCE: acceptance-must-be-a-named-policy // @ts-expect-error the four policies in rpc-acceptance-policies.ts are the whole vocabulary export const fenceInventedPolicy: RpcAcceptanceName = 'no-error-means-fine' @@ -71,7 +63,6 @@ const fenceTextReader: RpcCompatibleReader = (raw) => ({ salvage: { droppedPaths: [], droppedCount: 0 } }) -// FENCE: object-policy-reader-sees-an-object export const fenceObjectPolicyWrongReader: ObjectResultRpcDefinition< 'worktree.ps', 'text', @@ -82,26 +73,20 @@ export const fenceObjectPolicyWrongReader: ObjectResultRpcDefinition< method: 'worktree.ps', acceptance: 'object-result-or-null', barrier: 'on-settle', - consumes: [], - schedules: [], // @ts-expect-error the policy admits a non-null object, not the string this reader expects read: fenceTextReader } -// FENCE: define-rejects-a-mismatched-definition export const fenceDefineRejectsMismatch = defineRpcOperation({ name: 'fence.defineRejectsMismatch', method: 'worktree.ps', // @ts-expect-error no overload of defineRpcOperation pairs a probe with a payload reader acceptance: 'method-not-found-refusal', barrier: 'on-settle', - consumes: [], - schedules: [], // @ts-expect-error ... and the reader it would need is exactly what the probe overload bans read: workspaceRowsReader }) -// FENCE: method-must-exist-in-the-catalog // @ts-expect-error only generated catalog method names are addressable export const fenceUnknownMethod: RpcMethodName = 'worktree.nope' @@ -109,22 +94,18 @@ export const fenceUnknownMethod: RpcMethodName = 'worktree.nope' // coercing builders admit) are both wrong for a sender in opposite directions, so these pin // the two failures a regression to either one would reintroduce. -// FENCE: send-params-omit-a-defaulted-field // `query` and `limit` carry .default(), so a sender may leave them out. Under z.output both // read as required and this line stops compiling. export const fenceOmitsDefaultedField: RpcSendParams<'files.searchPaths'> = { worktree: 'w' } -// FENCE: send-params-reject-a-wrong-typed-field export const fenceRejectsWrongFieldType: RpcSendParams<'files.searchPaths'> = { // @ts-expect-error z.input of a z.unknown().transform builder admits any value; this does not worktree: 42 } -// FENCE: send-params-still-name-required-fields // @ts-expect-error `worktree` has neither a default nor an optional marker export const fenceKeepsRequiredField: RpcSendParams<'files.searchPaths'> = { query: 'x' } -// FENCE: send-params-never-tighten-the-parsed-shape // Catalog-wide: anything a handler could have been handed is something a sender may write. // A method that ever resolves tighter than its parsed shape lands in this union. declare const fenceTighterThanParsed: { @@ -132,7 +113,6 @@ declare const fenceTighterThanParsed: { }[RpcMethodName] & {} export const fenceNoTighterMethod: never = fenceTighterThanParsed -// FENCE: send-params-do-not-degenerate-to-unknown // z.input collapses every coercing builder to `unknown`. Only plugins.panelAction may be // unknown, because its schema is literally z.unknown(). declare const fenceUnknownParams: { @@ -141,21 +121,18 @@ declare const fenceUnknownParams: { export const fenceOnlyDeclaredUnknown: 'plugins.panelAction' = fenceUnknownParams export async function fenceBarrierAndParams(): Promise { - // FENCE: declared-barrier-cannot-be-moved-earlier await runRpcOperation( client, // @ts-expect-error this family interprets after all requests, so it has no on-settle run workspaceListAtBarrier, {} ) - // FENCE: on-settle-operation-cannot-defer-to-a-barrier startRpcOperation( client, // @ts-expect-error an on-settle family must not be parked behind someone else's barrier worktreePsProbe, {} ) - // FENCE: params-are-fixed-by-the-method await runRpcOperation( client, workspaceListOrNull, @@ -165,13 +142,11 @@ export async function fenceBarrierAndParams(): Promise { } export async function fenceVerdictTypes(): Promise { - // FENCE: probe-verdict-is-not-a-decoded-value // @ts-expect-error the probe's policy yields a boolean, not the other family's rows const rows: WorkspaceRows = await runRpcOperation(client, worktreePsProbe, {}) void rows } -// FENCE: manual-decoding-descriptor-needs-a-reader // @ts-expect-error the public descriptor also requires decoding, even without the factory export const fenceManualWithoutReader: RpcOperation< 'worktree.ps', @@ -183,12 +158,9 @@ export const fenceManualWithoutReader: RpcOperation< name: 'fence.manual', method: 'worktree.ps', acceptance: 'require-result-or-throw', - barrier: 'on-settle', - consumes: [], - schedules: [] + barrier: 'on-settle' } -// FENCE: broad-policy-still-requires-a-reader // @ts-expect-error widening the policy cannot disconnect it from its required reader export const fenceBroadWithoutReader: RpcOperation< 'worktree.ps', @@ -201,12 +173,9 @@ export const fenceBroadWithoutReader: RpcOperation< method: 'worktree.ps', acceptance: 'require-result-or-throw', barrier: 'on-settle', - consumes: [], - schedules: [], read: undefined } -// FENCE: object-descriptor-needs-a-reader // @ts-expect-error object acceptance must decode, just like require-result acceptance export const fenceObjectWithoutReader: RpcOperation< 'worktree.ps', @@ -218,7 +187,5 @@ export const fenceObjectWithoutReader: RpcOperation< name: 'fence.object', method: 'worktree.ps', acceptance: 'object-result-or-null', - barrier: 'on-settle', - consumes: [], - schedules: [] + barrier: 'on-settle' } diff --git a/mobile/src/transport/rpc-operation-contract.ts b/mobile/src/transport/rpc-operation-contract.ts index b6b6f970d0c..581506a97a7 100644 --- a/mobile/src/transport/rpc-operation-contract.ts +++ b/mobile/src/transport/rpc-operation-contract.ts @@ -73,10 +73,6 @@ export type RpcOperation< readonly name: string readonly method: Method readonly barrier: Barrier - /** Reply fields this family reads — the host contract a wire change has to respect. */ - readonly consumes: readonly string[] - /** Refresh/poll schedules this family owns; empty when it only runs on user intent. */ - readonly schedules: readonly string[] } & { [Policy in RpcAcceptanceName]: { readonly acceptance: Policy @@ -89,7 +85,7 @@ export type RpcOperation< // Internal interpreter view; public send APIs retain the policy/reader correlation. export type AnyRpcOperation = Pick< RpcOperation, - 'name' | 'method' | 'acceptance' | 'barrier' | 'consumes' | 'schedules' + 'name' | 'method' | 'acceptance' | 'barrier' > & { readonly read: RpcCompatibleReader | undefined } /** The verdict the declared policy yields. Not a per-call choice. */ @@ -117,8 +113,6 @@ type RpcOperationDefinition< name: string method: Method barrier: Barrier - consumes: readonly string[] - schedules: readonly string[] } export type RequireResultRpcDefinition< diff --git a/mobile/src/transport/rpc-operation-result-reader.test.ts b/mobile/src/transport/rpc-operation-result-reader.test.ts index fb51a3d6d34..0a5393352b3 100644 --- a/mobile/src/transport/rpc-operation-result-reader.test.ts +++ b/mobile/src/transport/rpc-operation-result-reader.test.ts @@ -120,8 +120,6 @@ describe('salvage through a descriptor', () => { method: 'worktree.ps', acceptance: 'require-result-or-throw', barrier: 'on-settle', - consumes: ['worktrees.id'], - schedules: [], read: salvagingReader }) diff --git a/mobile/src/transport/rpc-operation-test-families.ts b/mobile/src/transport/rpc-operation-test-families.ts index 8adde3565eb..73554b7a4f8 100644 --- a/mobile/src/transport/rpc-operation-test-families.ts +++ b/mobile/src/transport/rpc-operation-test-families.ts @@ -34,8 +34,6 @@ export const workspaceListOrThrow = defineRpcOperation({ method: 'worktree.ps', acceptance: 'require-result-or-throw', barrier: 'on-settle', - consumes: ['worktrees.id'], - schedules: [], read: workspaceRowsOrLegacyReader }) @@ -46,8 +44,6 @@ export const workspaceListOrNull = defineRpcOperation({ method: 'worktree.ps', acceptance: 'object-result-or-null', barrier: 'on-settle', - consumes: ['worktrees.id'], - schedules: [], read: workspaceRowsReader }) @@ -55,18 +51,14 @@ export const worktreePsProbe = defineRpcOperation({ name: 'test.worktreePsProbe', method: 'worktree.ps', acceptance: 'method-not-found-refusal', - barrier: 'on-settle', - consumes: [], - schedules: [] + barrier: 'on-settle' }) export const terminalStreamOpener = defineRpcOperation({ name: 'test.terminalStreamOpener', method: 'terminal.subscribe', acceptance: 'streaming-opener', - barrier: 'on-settle', - consumes: [], - schedules: [] + barrier: 'on-settle' }) export const workspaceListAtBarrier = defineRpcOperation({ @@ -74,8 +66,6 @@ export const workspaceListAtBarrier = defineRpcOperation({ method: 'worktree.ps', acceptance: 'require-result-or-throw', barrier: 'after-all-requests', - consumes: ['worktrees.id'], - schedules: ['test-poll'], read: workspaceRowsReader }) @@ -84,8 +74,6 @@ export const terminalListAtBarrier = defineRpcOperation({ method: 'terminal.list', acceptance: 'object-result-or-null', barrier: 'after-all-requests', - consumes: ['terminals'], - schedules: [], read: rpcResultVariant('terminals', z.object({ terminals: z.array(z.unknown()) })) }) @@ -93,9 +81,7 @@ export const worktreePsProbeAtBarrier = defineRpcOperation({ name: 'test.worktreePsProbeAtBarrier', method: 'worktree.ps', acceptance: 'method-not-found-refusal', - barrier: 'after-all-requests', - consumes: [], - schedules: [] + barrier: 'after-all-requests' }) export function rpcSuccess(result: unknown, streaming?: true): RpcResponse { diff --git a/mobile/src/transport/rpc-operation.test.ts b/mobile/src/transport/rpc-operation.test.ts index f7c011e85ee..91542b48528 100644 --- a/mobile/src/transport/rpc-operation.test.ts +++ b/mobile/src/transport/rpc-operation.test.ts @@ -1,5 +1,3 @@ -import { readFileSync } from 'node:fs' -import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' import type { RpcResponse } from './types' import { FakeSession } from './mobile-endpoint-supervisor-test-fakes' @@ -334,49 +332,4 @@ describe('a descriptor', () => { ;(workspaceListOrThrow as { barrier: string }).barrier = 'after-all-requests' }).toThrow(TypeError) }) - - it('publishes the fields it consumes and the schedules it owns', () => { - expect(workspaceListOrThrow.consumes).toEqual(['worktrees.id']) - expect(workspaceListOrThrow.schedules).toEqual([]) - expect(Object.isFrozen(workspaceListOrThrow.consumes)).toBe(true) - }) -}) - -// The fence itself is checked by `tsc --noEmit` (an expect-error directive that stops catching -// an error fails the typecheck). This pins its coverage so the file cannot be quietly gutted. -describe('the compile fence', () => { - const fenceSource = readFileSync( - fileURLToPath(new URL('./rpc-operation-compile-fence.ts', import.meta.url)), - 'utf8' - ) - const expectErrorDirective = `@ts-${'expect-error'}` - - it('still asserts every rejection it is meant to', () => { - const markers = [...fenceSource.matchAll(/\/\/ FENCE: (?[a-z-]+)/g)].map( - (match) => match.groups?.name - ) - - expect(markers).toEqual([ - 'variant-readers-cannot-be-empty', - 'probe-cannot-take-a-reader', - 'decoding-policy-needs-a-reader', - 'acceptance-must-be-a-named-policy', - 'object-policy-reader-sees-an-object', - 'define-rejects-a-mismatched-definition', - 'method-must-exist-in-the-catalog', - 'send-params-omit-a-defaulted-field', - 'send-params-reject-a-wrong-typed-field', - 'send-params-still-name-required-fields', - 'send-params-never-tighten-the-parsed-shape', - 'send-params-do-not-degenerate-to-unknown', - 'declared-barrier-cannot-be-moved-earlier', - 'on-settle-operation-cannot-defer-to-a-barrier', - 'params-are-fixed-by-the-method', - 'probe-verdict-is-not-a-decoded-value', - 'manual-decoding-descriptor-needs-a-reader', - 'broad-policy-still-requires-a-reader', - 'object-descriptor-needs-a-reader' - ]) - expect(fenceSource.split(expectErrorDirective).length - 1).toBe(17) - }) }) diff --git a/mobile/src/transport/rpc-operation.ts b/mobile/src/transport/rpc-operation.ts index b1607ac85c6..def072dac81 100644 --- a/mobile/src/transport/rpc-operation.ts +++ b/mobile/src/transport/rpc-operation.ts @@ -68,8 +68,6 @@ export function defineRpcOperation(definition: RpcOperationDefinitionInput): Any method: definition.method, acceptance: definition.acceptance, barrier: definition.barrier, - consumes: Object.freeze([...definition.consumes]), - schedules: Object.freeze([...definition.schedules]), // Why: classifyReply only ever hands a reader the payload its own policy admitted, so // the object policy's narrower parameter is sound to store as unknown. read: definition.read as RpcCompatibleReader | undefined