mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
test(mobile): ratchet the 201 unchecked RPC reply readers
Step 4 moved every call-site cast into an RpcOperation's `read`, but 201 of those readers still answer `compatible: true` for any payload: `rpcUncheckedPayloadReader` (163), `rpcReadUnchecked` (26 outside its own module) and `rpcUncheckedMemberReader` (12), across 42 files. The cast moved; it did not become true. Held as data with an AST boundary test, shaped on the raw-request-port ratchet: a file that is not listed fails, a listed file that no longer has one fails, and a count that rises fails. Only a call counts, so an import is not a reader and prose never is. No behaviour change: this commit adds a list and a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
Jinwoo-H
@@ -0,0 +1,180 @@
|
||||
import { readFileSync, readdirSync } from 'node:fs'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { extname, join, relative } from 'node:path'
|
||||
import ts from 'typescript'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
UNCHECKED_RPC_READERS,
|
||||
type UncheckedRpcReaderEntry
|
||||
} from './unchecked-rpc-reader-inventory'
|
||||
|
||||
/**
|
||||
* Ratchet for unchecked reply readers.
|
||||
*
|
||||
* `rpcUncheckedPayloadReader('x')` says nothing about the payload: it answers `compatible: true`
|
||||
* for a string, a null, an error envelope and the shape the consumer expects alike. The operation
|
||||
* then hands that value to a consumer typed as if it had been checked. This list is what the
|
||||
* count-down to zero is measured against, and this test is what makes it bind.
|
||||
*
|
||||
* Three failures, all of which mean "edit the list":
|
||||
* - a file holds an unchecked reader and is on the list of none,
|
||||
* - a listed file no longer holds one (stale entry — how allow-lists rot),
|
||||
* - a listed file's reader count went up.
|
||||
*
|
||||
* What this does NOT catch, all accepted:
|
||||
* - A hand-written `{ compatible: true, value: raw as T }` reader. Same hole, different bytes;
|
||||
* `unchecked-rpc-reader-inventory.ts` says so in prose because no AST rule separates a
|
||||
* projecting reader that validated its input from one that asserted.
|
||||
* - Whether a *checked* schema is any good. A `z.unknown()` reader counts as checked here and is
|
||||
* the right answer for a payload the consumer forwards opaquely; it is the wrong answer for one
|
||||
* it destructures, and only the consumer trace tells the two apart.
|
||||
* - Test files. `*.test.ts(x)` is not scanned: a suite asserting on an unchecked reader is
|
||||
* testing the helper, and a test does not ship.
|
||||
*/
|
||||
|
||||
const mobileRoot = fileURLToPath(new URL('../..', import.meta.url))
|
||||
const scannedRoots = ['app', 'src'].map((directory) => join(mobileRoot, directory))
|
||||
const sourceExtensions = new Set(['.js', '.jsx', '.ts', '.tsx'])
|
||||
|
||||
/** The three helpers, and the module that defines them — it is not an offender. */
|
||||
const UNCHECKED_READER_NAMES = new Set([
|
||||
'rpcUncheckedPayloadReader',
|
||||
'rpcUncheckedMemberReader',
|
||||
'rpcReadUnchecked'
|
||||
])
|
||||
const SELF_FILES = new Set([
|
||||
'src/transport/rpc-reader-payload.ts',
|
||||
'src/transport/unchecked-rpc-reader-inventory.ts'
|
||||
])
|
||||
|
||||
function sourceFiles(directory: string): string[] {
|
||||
return readdirSync(directory, { withFileTypes: true }).flatMap((entry) => {
|
||||
const path = join(directory, entry.name)
|
||||
if (entry.isDirectory()) {
|
||||
return entry.name === 'node_modules' ? [] : sourceFiles(path)
|
||||
}
|
||||
return [path]
|
||||
})
|
||||
}
|
||||
|
||||
function parse(path: string, source: string): ts.SourceFile {
|
||||
const extension = extname(path)
|
||||
return ts.createSourceFile(
|
||||
path,
|
||||
source,
|
||||
ts.ScriptTarget.Latest,
|
||||
true,
|
||||
extension === '.tsx' || extension === '.jsx' ? ts.ScriptKind.TSX : ts.ScriptKind.TS
|
||||
)
|
||||
}
|
||||
|
||||
/** How many unchecked readers this file builds. Only a call counts: an import is not a reader. */
|
||||
export function uncheckedReaderCount(path: string, source: string): number {
|
||||
let readers = 0
|
||||
const visit = (node: ts.Node): void => {
|
||||
if (
|
||||
ts.isCallExpression(node) &&
|
||||
ts.isIdentifier(node.expression) &&
|
||||
UNCHECKED_READER_NAMES.has(node.expression.text)
|
||||
) {
|
||||
readers += 1
|
||||
}
|
||||
ts.forEachChild(node, visit)
|
||||
}
|
||||
visit(parse(path, source))
|
||||
return readers
|
||||
}
|
||||
|
||||
const scanned = scannedRoots
|
||||
.flatMap(sourceFiles)
|
||||
.filter((path) => sourceExtensions.has(extname(path)))
|
||||
.filter((path) => !/\.test\.tsx?$/.test(path))
|
||||
.map((path) => relative(mobileRoot, path).split(/[/\\]/).join('/'))
|
||||
.filter((file) => !SELF_FILES.has(file))
|
||||
|
||||
const observed = new Map(
|
||||
scanned
|
||||
.map(
|
||||
(file) =>
|
||||
[
|
||||
file,
|
||||
uncheckedReaderCount(join(mobileRoot, file), readFileSync(join(mobileRoot, file), 'utf8'))
|
||||
] as const
|
||||
)
|
||||
.filter(([, readers]) => readers > 0)
|
||||
)
|
||||
|
||||
const inventory: readonly UncheckedRpcReaderEntry[] = UNCHECKED_RPC_READERS
|
||||
|
||||
describe('unchecked RPC reader boundary', () => {
|
||||
const probe = join(mobileRoot, 'src', 'transport', 'probe.ts')
|
||||
|
||||
it('counts every shape that builds an unchecked reader', () => {
|
||||
expect(uncheckedReaderCount(probe, "read: rpcUncheckedPayloadReader('x')")).toBe(1)
|
||||
expect(uncheckedReaderCount(probe, "read: rpcUncheckedMemberReader('x', 'entries')")).toBe(1)
|
||||
expect(uncheckedReaderCount(probe, "return rpcReadUnchecked('x', raw)")).toBe(1)
|
||||
expect(uncheckedReaderCount(probe, "a(rpcReadUnchecked('x', rpcReadUnchecked('y', 1)))")).toBe(
|
||||
2
|
||||
)
|
||||
})
|
||||
|
||||
it('does not count prose, an import or a checked reader', () => {
|
||||
expect(uncheckedReaderCount(probe, '// rpcUncheckedPayloadReader is the old shape')).toBe(0)
|
||||
expect(uncheckedReaderCount(probe, '/* rpcReadUnchecked */ export const x = 1')).toBe(0)
|
||||
expect(
|
||||
uncheckedReaderCount(
|
||||
probe,
|
||||
"import { rpcUncheckedPayloadReader } from './rpc-reader-payload'"
|
||||
)
|
||||
).toBe(0)
|
||||
expect(uncheckedReaderCount(probe, "read: rpcResultVariant('x', schema)")).toBe(0)
|
||||
})
|
||||
|
||||
it('scans a plausible number of files', () => {
|
||||
// A broken root or extension filter would make every check below vacuously pass. The floor on
|
||||
// the offender count comes down with the list, so a successful migration step does not fail it.
|
||||
expect(scanned.length).toBeGreaterThan(400)
|
||||
expect(observed.size).toBeGreaterThan(20)
|
||||
})
|
||||
|
||||
it('lists each file once', () => {
|
||||
const seen = inventory.map((entry) => entry.file)
|
||||
expect(seen.filter((file, index) => seen.indexOf(file) !== index)).toEqual([])
|
||||
})
|
||||
|
||||
it('has no unlisted file holding an unchecked reader', () => {
|
||||
const listed = new Set(inventory.map((entry) => entry.file))
|
||||
const unlisted = [...observed.keys()].filter((file) => !listed.has(file))
|
||||
expect(
|
||||
unlisted,
|
||||
'A new operation validates its reply with rpcResultVariant. Nothing may be added to unchecked-rpc-reader-inventory.ts.'
|
||||
).toEqual([])
|
||||
})
|
||||
|
||||
it('has no stale inventory entry', () => {
|
||||
const stale = inventory.filter((entry) => !observed.has(entry.file))
|
||||
expect(
|
||||
stale.map((entry) => entry.file),
|
||||
'File holds no unchecked reader — delete its line from unchecked-rpc-reader-inventory.ts.'
|
||||
).toEqual([])
|
||||
})
|
||||
|
||||
it('has no inventory entry whose file gained readers', () => {
|
||||
const grown = inventory
|
||||
.filter((entry) => (observed.get(entry.file) ?? 0) > entry.readers)
|
||||
.map((entry) => `${entry.file}: listed ${entry.readers}, found ${observed.get(entry.file)}`)
|
||||
expect(grown, 'The counts are a ceiling. Validate the new reply with a schema.').toEqual([])
|
||||
})
|
||||
|
||||
it('reports a count that has fallen so the entry can be lowered', () => {
|
||||
const overstated = inventory
|
||||
.filter(
|
||||
(entry) => observed.has(entry.file) && (observed.get(entry.file) ?? 0) < entry.readers
|
||||
)
|
||||
.map((entry) => `${entry.file}: listed ${entry.readers}, found ${observed.get(entry.file)}`)
|
||||
expect(
|
||||
overstated,
|
||||
'Fewer unchecked readers than listed — lower the count so the ratchet holds.'
|
||||
).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,93 @@
|
||||
/**
|
||||
* Every RpcOperation reader that re-types its reply instead of validating it, held as data.
|
||||
*
|
||||
* A reader is unchecked when it answers `compatible: true` for every payload a byte can carry:
|
||||
* a call to `rpcUncheckedPayloadReader`, `rpcUncheckedMemberReader` or `rpcReadUnchecked` in
|
||||
* rpc-reader-payload.ts. Step 4 moved the call-site cast into the operation's `read`; it did not
|
||||
* make the cast true. A malformed reply still reaches the consumer as the declared type and fails
|
||||
* somewhere downstream — a property read on null, a `.map` on a string, a rendered `undefined` —
|
||||
* with nothing naming the reply as the cause.
|
||||
*
|
||||
* The count is per file and is a ceiling, not a target: unchecked-rpc-reader-boundary.test.ts fails
|
||||
* on a file that is not listed, on a listed file that no longer has one, and on a listed file whose
|
||||
* count went up. Replacing a reader with `rpcResultVariant(variant, schema)` lowers its line; the
|
||||
* list only shrinks.
|
||||
*
|
||||
* Two holes this list does not close, both deliberate:
|
||||
* - A hand-written reader that returns `{ compatible: true, ... }` without going through those
|
||||
* three helpers is not counted. It is the same hole with different bytes; the AST cannot tell
|
||||
* a projecting reader that validated its input from one that did not.
|
||||
* - `rpcPayloadMember` at a call site outside a reader. That is an unchecked member read, not a
|
||||
* reader, and it is fenced by the raw-port inventory instead.
|
||||
*/
|
||||
export type UncheckedRpcReaderEntry = {
|
||||
readonly file: string
|
||||
readonly readers: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Files holding at least one unchecked reader, grouped by the feature area that owns them.
|
||||
*
|
||||
* The reason is shared by every line and is stated once here instead of 42 times: the reply has no
|
||||
* schema, so the operation declares what the payload is by assertion. Writing one schema per
|
||||
* consumed member — required exactly where the consumer reads it unguarded, optional everywhere
|
||||
* else, never `.strict()` — turns the assertion into a check and deletes the line.
|
||||
*/
|
||||
export const UNCHECKED_RPC_READERS: readonly UncheckedRpcReaderEntry[] = [
|
||||
// agent-history
|
||||
{ file: 'src/agent-history/mobile-agent-history-operations.ts', readers: 6 },
|
||||
// browser
|
||||
{ file: 'src/browser/mobile-browser-command-operations.ts', readers: 1 },
|
||||
// components
|
||||
{ file: 'src/components/codex-reset-credit-capability-operations.ts', readers: 1 },
|
||||
{ file: 'src/components/codex-reset-credit-consume-operations.ts', readers: 1 },
|
||||
{ file: 'src/components/new-workspace-operations.ts', readers: 2 },
|
||||
// dictation
|
||||
{ file: 'src/dictation/mobile-dictation-operations.ts', readers: 8 },
|
||||
// files
|
||||
{ file: 'src/files/mobile-file-explorer-operations.ts', readers: 2 },
|
||||
{ file: 'src/files/mobile-file-ownership-operations.ts', readers: 2 },
|
||||
{ file: 'src/files/mobile-file-preview-operations.ts', readers: 6 },
|
||||
{ file: 'src/files/mobile-file-tab-doc-operations.ts', readers: 3 },
|
||||
// home
|
||||
{ file: 'src/home/mobile-home-host-operations.ts', readers: 2 },
|
||||
// host-screen
|
||||
{ file: 'src/host-screen/host-screen-operations.ts', readers: 8 },
|
||||
// notifications
|
||||
{ file: 'src/notifications/mobile-push-delivery-test-operations.ts', readers: 1 },
|
||||
{ file: 'src/notifications/mobile-push-registration-operations.ts', readers: 2 },
|
||||
{ file: 'src/notifications/push-dismissal-operations.ts', readers: 1 },
|
||||
// session
|
||||
{ file: 'src/session/github-pr-mutation-operations.ts', readers: 4 },
|
||||
{ file: 'src/session/github-pr-read-operations.ts', readers: 8 },
|
||||
{ file: 'src/session/mobile-clipboard-image-operations.ts', readers: 5 },
|
||||
{ file: 'src/session/mobile-diff-review-git-operations.ts', readers: 2 },
|
||||
{ file: 'src/session/mobile-diff-review-operations.ts', readers: 3 },
|
||||
{ file: 'src/session/mobile-review-terminal-operations.ts', readers: 3 },
|
||||
{ file: 'src/session/mobile-session-launch-operations.ts', readers: 7 },
|
||||
{ file: 'src/session/mobile-session-read-operations.ts', readers: 10 },
|
||||
{ file: 'src/session/mobile-session-write-operations.ts', readers: 8 },
|
||||
// source-control
|
||||
{ file: 'src/source-control/mobile-git-mutation-operations.ts', readers: 7 },
|
||||
{ file: 'src/source-control/mobile-git-read-operations.ts', readers: 5 },
|
||||
{ file: 'src/source-control/mobile-hosted-review-operations.ts', readers: 2 },
|
||||
{ file: 'src/source-control/mobile-source-file-open-operations.ts', readers: 2 },
|
||||
{ file: 'src/source-control/mobile-worktree-metadata-operations.ts', readers: 1 },
|
||||
// tasks
|
||||
{ file: 'src/tasks/mobile-task-item-comment-operations.ts', readers: 7 },
|
||||
{ file: 'src/tasks/mobile-task-item-detail-operations.ts', readers: 8 },
|
||||
{ file: 'src/tasks/mobile-task-item-state-operations.ts', readers: 17 },
|
||||
{ file: 'src/tasks/mobile-task-list-operations.ts', readers: 6 },
|
||||
{ file: 'src/tasks/mobile-task-project-board-operations.ts', readers: 17 },
|
||||
{ file: 'src/tasks/mobile-task-runtime-operations.ts', readers: 7 },
|
||||
{ file: 'src/tasks/mobile-task-source-search-operations.ts', readers: 7 },
|
||||
{ file: 'src/tasks/mobile-workspace-create-operations.ts', readers: 4 },
|
||||
{ file: 'src/tasks/mobile-workspace-source-operations.ts', readers: 7 },
|
||||
// terminal
|
||||
{ file: 'src/terminal/mobile-terminal-operations.ts', readers: 3 },
|
||||
// transport
|
||||
{ file: 'src/transport/host-status-probe-operations.ts', readers: 1 },
|
||||
{ file: 'src/transport/mobile-relay-pairing-operations.ts', readers: 2 },
|
||||
// worktree
|
||||
{ file: 'src/worktree/worktree-catalog-operations.ts', readers: 2 }
|
||||
]
|
||||
Reference in New Issue
Block a user