mirror of
https://github.com/stablyai/orca.git
synced 2026-10-02 00:02:05 +00:00
`anti-slop/no-reflect-get` rejects every call to `Reflect.get`. The
reflective read bypasses ordinary property access and throws away the
type evidence the compiler would otherwise give you: the result is
`any`/`unknown` with no narrowing, so a typo in the key or a shape drift
in the source object is invisible until runtime. The rule's remedy is to
parse dynamic input into a named domain type (or narrow it with `in`)
and then read the field normally.
Baseline: 86 violations across 67 files. Now zero unsuppressed
violations under
`npx oxlint --config config/oxlint-anti-slop.json --ignore-pattern 'config/oxlint-plugins/anti-slop/**' src config tests mobile`.
Fix pattern
-----------
44 of the 86 were rewritten. The dominant shape was an `unknown` value
read through `Reflect.get` right after a `typeof === 'object'` guard;
those became `in`-narrowed property access, which TypeScript checks:
- Reflect.get(value, 'agents')
+ 'agents' in value ? value.agents : null
Two further shapes:
- `Reflect.get(Object(x), 'k')` on a possibly-primitive envelope became a
small named reader that boxes once and indexes a
`Record<string, unknown>` (`settingsField` in
mobile/src/transport/settings-read-operations.ts).
- Tests reaching into private state moved to TypeScript's checked
bracket-index escape hatch (`runtime['layoutQueues']`), or to a
documented read-only accessor on the owning class
(`SearchSubprocessLineAccumulator.retainedCapacityBytes()`,
`CodexSubagentExecutions.retentionSizes()`).
No type assertion was added anywhere: the diff contains zero net-new
`as` casts, `as any`, `as unknown as`, `@ts-ignore`, or
`@ts-expect-error`, so nothing was laundered into the sibling
assertion rules.
Suppressions
------------
42x `// oxlint-disable-next-line anti-slop/no-reflect-get` across 38
files. Every one is the default-forward branch of a `Proxy` `get` trap:
get(target, property, receiver) {
...
return Reflect.get(target, property, receiver)
}
`Reflect.get(target, property, receiver)` is the only construct that
forwards with correct `receiver` semantics; `target[property]` invokes
an accessor with the wrong `this` and silently breaks getters that read
sibling state. There is no typed alternative, so these are suppressed
rather than rewritten.
3x `// oxlint-disable-next-line typescript-eslint/consistent-type-definitions
-- declaration merging requires interface` in
tests/e2e/github-url-smart-input-transition.spec.ts,
tests/e2e/linear-url-workspace-entry.spec.ts, and
tests/e2e/worktree-active-delete-scroll-position.spec.ts. Replacing
`Reflect.get(window, 'x')` with typed `window.x` requires a
`declare global { interface Window }` block, and `interface` is
mandatory for declaration merging. Matches the existing convention at
tests/e2e/helpers/runtime-types.ts:63.
1x `// eslint-disable-next-line no-var -- main-process gate handle for
this spec` in tests/e2e/project-group-creation-visibility.spec.ts, for
the same reason a `var` global is needed to type the handle. Matches
tests/e2e/agent-session-log-tail-stability.spec.ts:24.
Also updates two source-text anchors in mobile's rpc-recording mutation
harness (mobile/src/test-support/rpc-recording/operation-mutations.ts
and recording-runner.test.ts), which pin the exact text of the rewritten
line in settings-read-operations.ts and would otherwise fail with
"Mutant anchor matched 0 sites, expected 1".
209 lines
7.5 KiB
TypeScript
209 lines
7.5 KiB
TypeScript
import type { Socket } from 'node:net'
|
|
import { encodeNdjson } from './ndjson'
|
|
import {
|
|
DAEMON_UNAVAILABLE_RECONNECT_MESSAGE,
|
|
DaemonConnectionLostError,
|
|
DaemonRequestTimeoutError
|
|
} from './types'
|
|
import { isTerminalAttachCanceledMessage } from './daemon-errors'
|
|
import type { DaemonPendingRequests } from './daemon-client-pending-requests'
|
|
|
|
type DaemonRpcRequestOptions = {
|
|
socket: Socket
|
|
pendingRequests: DaemonPendingRequests
|
|
id: string
|
|
type: string
|
|
payload: unknown
|
|
timeoutMs: number
|
|
signal?: AbortSignal
|
|
/**
|
|
* How long to keep waiting after the daemon says it could not match a cancel.
|
|
* A create that already published a result still has a response in flight, but
|
|
* an attach-only request has nothing coming, so the wait must be bounded.
|
|
*/
|
|
unmatchedCancelGraceMs: number
|
|
/**
|
|
* Escalation of last resort: called only when the cancel RPC could not be put on
|
|
* the wire at all. Never called for a cancel the daemon answered with an error,
|
|
* or one that timed out — those say nothing about the sibling sessions sharing
|
|
* this connection. A cancel that timed out is instead reported through
|
|
* `wedgedDaemonError` on this request alone.
|
|
*/
|
|
onCreateCancellationFailure: () => void
|
|
settleCreateCancellation: (sessionId: string, requestId: string) => Promise<{ canceled: boolean }>
|
|
}
|
|
|
|
/**
|
|
* Why: the request timed out and so did the cancel sent to clean it up, so the daemon
|
|
* is wedged with its socket still open — no close event, no disconnect, and nothing
|
|
* else in the client notices (#8689 is the same shape). Re-message it to the one
|
|
* string `isDaemonGoneError` matches, so the caller's retry can respawn the daemon
|
|
* instead of retrying against a daemon that will never answer. Aborts are excluded:
|
|
* the caller asked to stop, not to retry.
|
|
*/
|
|
function wedgedDaemonError(requestError: Error, cancelError: unknown): Error | null {
|
|
if (
|
|
!(requestError instanceof DaemonRequestTimeoutError) ||
|
|
!(cancelError instanceof DaemonRequestTimeoutError)
|
|
) {
|
|
return null
|
|
}
|
|
const wedged = new DaemonRequestTimeoutError(DAEMON_UNAVAILABLE_RECONNECT_MESSAGE)
|
|
wedged.cause = requestError
|
|
return wedged
|
|
}
|
|
|
|
export function requestDaemonRpc<T>(opts: DaemonRpcRequestOptions): Promise<T> {
|
|
// A stalled event loop can deliver a daemon cancellation before its overdue timer runs.
|
|
const { payload, type } = opts
|
|
const createTimeoutError = (): DaemonRequestTimeoutError =>
|
|
new DaemonRequestTimeoutError(`Request ${type} timed out after ${opts.timeoutMs}ms`)
|
|
const createSessionId =
|
|
type === 'createOrAttach' &&
|
|
payload !== null &&
|
|
typeof payload === 'object' &&
|
|
'sessionId' in payload
|
|
? payload.sessionId
|
|
: null
|
|
const requestPayload =
|
|
type === 'createOrAttach' && payload !== null && typeof payload === 'object'
|
|
? {
|
|
...payload,
|
|
cancelAfterMs: Math.max(1, opts.timeoutMs + opts.unmatchedCancelGraceMs)
|
|
}
|
|
: payload
|
|
const encoded = encodeNdjson({
|
|
id: opts.id,
|
|
type,
|
|
...(requestPayload !== undefined ? { payload: requestPayload } : {})
|
|
})
|
|
const clientDeadlineMs = performance.now() + opts.timeoutMs
|
|
|
|
return new Promise<T>((resolve, reject) => {
|
|
let sent = false
|
|
let cancellationStarted = false
|
|
let settled = false
|
|
let unmatchedCancelTimer: NodeJS.Timeout | null = null
|
|
// Why: our cancel makes the daemon reject the request too (a queued create
|
|
// abandoning an aborted wait). Callers key recovery off `client_disconnected`,
|
|
// so letting that race pick the message rolls back terminals it should keep.
|
|
// Scoped to the daemon's cancellation reply: a real disconnect still wins.
|
|
let cancellationError: Error | null = null
|
|
const removeAbortListener = (): void => opts.signal?.removeEventListener('abort', onAbort)
|
|
const clearTimers = (): void => {
|
|
clearTimeout(timer)
|
|
if (unmatchedCancelTimer) {
|
|
clearTimeout(unmatchedCancelTimer)
|
|
unmatchedCancelTimer = null
|
|
}
|
|
}
|
|
const rejectAndDrop = (error: Error): void => {
|
|
if (settled) {
|
|
return
|
|
}
|
|
settled = true
|
|
opts.pendingRequests.drop(opts.id)
|
|
removeAbortListener()
|
|
clearTimers()
|
|
reject(error)
|
|
}
|
|
// Unmatched or unconfirmed cancel: a create that already published a result will
|
|
// still answer, so keep waiting — but only for a bounded window, or an
|
|
// attach-only request queued behind a hung create never settles at all.
|
|
const awaitLateResponseThenReject = (error: Error): void => {
|
|
if (settled || unmatchedCancelTimer) {
|
|
return
|
|
}
|
|
unmatchedCancelTimer = setTimeout(() => rejectAndDrop(error), opts.unmatchedCancelGraceMs)
|
|
unmatchedCancelTimer.unref?.()
|
|
}
|
|
const cancelCreate = (error: Error): void => {
|
|
if (cancellationStarted) {
|
|
return
|
|
}
|
|
cancellationStarted = true
|
|
cancellationError = error
|
|
clearTimeout(timer)
|
|
if (!sent || typeof createSessionId !== 'string') {
|
|
rejectAndDrop(error)
|
|
return
|
|
}
|
|
void opts
|
|
.settleCreateCancellation(createSessionId, opts.id)
|
|
.then((result) => {
|
|
if (result.canceled) {
|
|
rejectAndDrop(error)
|
|
return
|
|
}
|
|
awaitLateResponseThenReject(error)
|
|
})
|
|
.catch((cancelError: unknown) => {
|
|
if (settled) {
|
|
return
|
|
}
|
|
// Why: a cancel the daemon refused (v1-v10 answer 'Unknown request type')
|
|
// or that blew its own 5s timeout (busy event loop, e.g. an unreachable UNC
|
|
// share) proves nothing about the other sessions on this connection, so it
|
|
// must not tear it down. Only an undeliverable cancel does. Unrecognized
|
|
// errors deliberately fall through to the bounded wait: the fail-safe
|
|
// direction is leaving siblings alone. A wedged daemon is still reported —
|
|
// on this request only — via wedgedDaemonError.
|
|
if (cancelError instanceof DaemonConnectionLostError) {
|
|
opts.onCreateCancellationFailure()
|
|
return
|
|
}
|
|
awaitLateResponseThenReject(wedgedDaemonError(error, cancelError) ?? error)
|
|
})
|
|
}
|
|
const timer = setTimeout(() => {
|
|
const error = createTimeoutError()
|
|
if (typeof createSessionId === 'string') {
|
|
cancelCreate(error)
|
|
} else {
|
|
rejectAndDrop(error)
|
|
}
|
|
}, opts.timeoutMs)
|
|
const onAbort = (): void => {
|
|
removeAbortListener()
|
|
cancelCreate(new Error('client_disconnected'))
|
|
}
|
|
|
|
opts.pendingRequests.add(opts.id, {
|
|
resolve: (value) => {
|
|
settled = true
|
|
removeAbortListener()
|
|
clearTimers()
|
|
resolve(value as T)
|
|
},
|
|
reject: (error) => {
|
|
settled = true
|
|
removeAbortListener()
|
|
clearTimers()
|
|
reject(
|
|
isTerminalAttachCanceledMessage(error.message)
|
|
? (cancellationError ??
|
|
(type === 'createOrAttach' && performance.now() >= clientDeadlineMs
|
|
? createTimeoutError()
|
|
: error))
|
|
: error
|
|
)
|
|
},
|
|
timer
|
|
})
|
|
|
|
opts.signal?.addEventListener('abort', onAbort, { once: true })
|
|
if (opts.signal?.aborted) {
|
|
onAbort()
|
|
return
|
|
}
|
|
try {
|
|
opts.socket.write(encoded)
|
|
sent = true
|
|
} catch (err) {
|
|
// Why: an unwrapped throw here leaks the pending entry and its timer, and the
|
|
// raw error would be indistinguishable from a daemon refusal.
|
|
rejectAndDrop(new DaemonConnectionLostError(err instanceof Error ? err.message : String(err)))
|
|
}
|
|
})
|
|
}
|