mirror of
https://github.com/stablyai/orca.git
synced 2026-10-27 16:02:42 +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".
371 lines
13 KiB
TypeScript
371 lines
13 KiB
TypeScript
import { mkdtemp, rm } from 'node:fs/promises'
|
|
import { tmpdir } from 'node:os'
|
|
import { join } from 'node:path'
|
|
import { afterEach, describe, expect, it, vi } from 'vitest'
|
|
import type { AgentSessionJournalIdentity } from '../../shared/agent-session-journal-types'
|
|
import { agentSessionJournalCloseRetries } from '../native-chat/agent-session-journal/journal-close-retry'
|
|
import { createTrackedJournalOpener } from '../native-chat/agent-session-journal/journal-store-test-open'
|
|
import type {
|
|
AgentSessionClaimStatus,
|
|
AgentSessionExecutionLocation,
|
|
AgentSessionProcessIdentity,
|
|
AgentSessionRecord
|
|
} from '../../shared/agent-session-record'
|
|
import { __setWindowsProcessTreeLoaderForTests } from '../windows/windows-process-table'
|
|
import {
|
|
createStructuredAgentSessionOwnerProbe,
|
|
createStructuredAgentSessionOwnerProbes
|
|
} from './structured-agent-session-owner-probe'
|
|
import {
|
|
ensureStructuredAgentSessionHost,
|
|
hasPersistedStructuredAgentSessionStore,
|
|
stopStructuredAgentSessionRuntime
|
|
} from './structured-agent-session-runtime'
|
|
|
|
const HOST_ID = 'local'
|
|
|
|
function record(
|
|
ownerProcess: AgentSessionProcessIdentity | null,
|
|
lease: {
|
|
processlessAt?: number | null
|
|
reservedSpawnToken?: string | null
|
|
claimStatus?: AgentSessionClaimStatus
|
|
runtimeFence?: number
|
|
} = {}
|
|
): AgentSessionRecord {
|
|
return {
|
|
sessionId: 'session-1',
|
|
providerHandleChain: [],
|
|
lease: {
|
|
ownerProcess,
|
|
reservedSpawnToken: null,
|
|
claimStatus: 'released',
|
|
runtimeFence: 3,
|
|
...lease
|
|
}
|
|
} as unknown as AgentSessionRecord
|
|
}
|
|
|
|
const OWNER: AgentSessionProcessIdentity = {
|
|
hostId: HOST_ID,
|
|
pid: 4242,
|
|
processStartTimeMs: 1_700_000_000_000,
|
|
spawnToken: 'token-1'
|
|
}
|
|
|
|
const deadProbe = () => vi.fn(async () => ({ outcome: 'pid-absent' }) as const)
|
|
|
|
describe('structured agent-session store presence', () => {
|
|
it('stops after finding the durable primary store', () => {
|
|
const fileExists = vi.fn(() => true)
|
|
|
|
expect(hasPersistedStructuredAgentSessionStore('/profile', fileExists)).toBe(true)
|
|
expect(fileExists).toHaveBeenCalledOnce()
|
|
expect(fileExists).toHaveBeenCalledWith(
|
|
join('/profile', 'agent-sessions', 'agent-sessions.json')
|
|
)
|
|
})
|
|
|
|
it('checks the durable backup when the primary store is absent', () => {
|
|
const fileExists = vi.fn((path: string) => path.endsWith('.bak'))
|
|
|
|
expect(hasPersistedStructuredAgentSessionStore('/profile', fileExists)).toBe(true)
|
|
expect(fileExists).toHaveBeenNthCalledWith(
|
|
1,
|
|
join('/profile', 'agent-sessions', 'agent-sessions.json')
|
|
)
|
|
expect(fileExists).toHaveBeenNthCalledWith(
|
|
2,
|
|
join('/profile', 'agent-sessions', 'agent-sessions.json.bak')
|
|
)
|
|
})
|
|
|
|
it('reports a fresh profile absent after two bounded presence checks', () => {
|
|
const fileExists = vi.fn(() => false)
|
|
|
|
expect(hasPersistedStructuredAgentSessionStore('/profile', fileExists)).toBe(false)
|
|
expect(fileExists).toHaveBeenCalledTimes(2)
|
|
})
|
|
})
|
|
|
|
describe('structured agent-session owner probe', () => {
|
|
it('probes an owner this host spawned', async () => {
|
|
const probe = deadProbe()
|
|
const result = await createStructuredAgentSessionOwnerProbe(HOST_ID, probe)(record(OWNER))
|
|
|
|
expect(probe).toHaveBeenCalledWith({
|
|
identity: OWNER,
|
|
deps: { readEchoedSpawnToken: expect.any(Function) }
|
|
})
|
|
expect(result).toEqual({ outcome: 'pid-absent' })
|
|
})
|
|
|
|
it('reads the process table once for many local owners', async () => {
|
|
const secondOwner = { ...OWNER, pid: 5252, spawnToken: 'token-2' }
|
|
const probeMany = vi.fn(async () => [
|
|
{ outcome: 'identity-matched' as const, matchedOn: ['process-start-time' as const] },
|
|
{ outcome: 'pid-absent' as const }
|
|
])
|
|
const probeOne = vi.fn(async () => ({ outcome: 'indeterminate' as const, reason: 'unused' }))
|
|
const records = [
|
|
record(OWNER),
|
|
{ ...record(secondOwner), sessionId: 'session-2' }
|
|
] as AgentSessionRecord[]
|
|
|
|
const results = await createStructuredAgentSessionOwnerProbes(
|
|
HOST_ID,
|
|
probeMany,
|
|
probeOne
|
|
)(records)
|
|
|
|
expect(probeMany).toHaveBeenCalledOnce()
|
|
expect(probeMany).toHaveBeenCalledWith({
|
|
identities: [OWNER, secondOwner],
|
|
deps: { readEchoedSpawnToken: expect.any(Function) }
|
|
})
|
|
expect(probeOne).not.toHaveBeenCalled()
|
|
expect(results.get('session-1')?.outcome).toBe('identity-matched')
|
|
expect(results.get('session-2')).toEqual({ outcome: 'pid-absent' })
|
|
})
|
|
|
|
it('refuses to probe an owner on another host, whose pid means nothing here', async () => {
|
|
const probe = deadProbe()
|
|
const result = await createStructuredAgentSessionOwnerProbe(
|
|
HOST_ID,
|
|
probe
|
|
)(record({ ...OWNER, hostId: 'ssh:build-box' }))
|
|
|
|
expect(probe).not.toHaveBeenCalled()
|
|
expect(result.outcome).toBe('indeterminate')
|
|
})
|
|
|
|
it('leaves a reservation whose spawn token is still live on this host latched', async () => {
|
|
const probe = deadProbe()
|
|
const result = await createStructuredAgentSessionOwnerProbe(HOST_ID, probe, async () => [9001])(
|
|
record(null, { claimStatus: 'reserved', reservedSpawnToken: 'token-1' })
|
|
)
|
|
|
|
// Evicting here would put a second writer on a live Codex thread.
|
|
expect(result.outcome).toBe('indeterminate')
|
|
})
|
|
|
|
it('leaves a reservation latched on a host that cannot enumerate spawn tokens', async () => {
|
|
const result = await createStructuredAgentSessionOwnerProbe(
|
|
HOST_ID,
|
|
deadProbe(),
|
|
async () => null
|
|
)(record(null, { claimStatus: 'reserved', reservedSpawnToken: 'token-1' }))
|
|
|
|
expect(result.outcome).toBe('indeterminate')
|
|
})
|
|
|
|
it('frees a reservation once the host proves no process carries its token', async () => {
|
|
const result = await createStructuredAgentSessionOwnerProbe(
|
|
HOST_ID,
|
|
deadProbe(),
|
|
async () => []
|
|
)(record(null, { claimStatus: 'reserved', reservedSpawnToken: 'token-1' }))
|
|
|
|
expect(result).toEqual({ outcome: 'reservation-unused' })
|
|
})
|
|
|
|
it('frees a lease that names neither an owner nor a spawn token', async () => {
|
|
const probe = deadProbe()
|
|
const scan = vi.fn(async () => [] as number[])
|
|
// Nothing was ever minted that a child could be carrying, so no scan is even needed;
|
|
// answering `indeterminate` here is what latches every released record into recovery.
|
|
const result = await createStructuredAgentSessionOwnerProbe(HOST_ID, probe, scan)(record(null))
|
|
|
|
expect(probe).not.toHaveBeenCalled()
|
|
expect(scan).not.toHaveBeenCalled()
|
|
expect(result).toEqual({ outcome: 'reservation-unused' })
|
|
})
|
|
|
|
it('still refuses a reservation that recorded no token to scan for', async () => {
|
|
const result = await createStructuredAgentSessionOwnerProbe(
|
|
HOST_ID,
|
|
deadProbe(),
|
|
async () => []
|
|
)(record(null, { claimStatus: 'reserved' }))
|
|
|
|
expect(result.outcome).toBe('indeterminate')
|
|
})
|
|
|
|
it('releases only a reservation carrying durable pre-spawn proof', async () => {
|
|
const probe = deadProbe()
|
|
const result = await createStructuredAgentSessionOwnerProbe(
|
|
HOST_ID,
|
|
probe
|
|
)(record(null, { processlessAt: 1_800_000_000_000, claimStatus: 'reserved' }))
|
|
|
|
expect(probe).not.toHaveBeenCalled()
|
|
expect(result).toEqual({ outcome: 'reservation-unused' })
|
|
})
|
|
})
|
|
|
|
describe('structured agent-session runtime install', () => {
|
|
let stateDirectory: string | null = null
|
|
|
|
afterEach(async () => {
|
|
await stopStructuredAgentSessionRuntime()
|
|
if (stateDirectory) {
|
|
await rm(stateDirectory, { recursive: true, force: true })
|
|
stateDirectory = null
|
|
}
|
|
vi.restoreAllMocks()
|
|
})
|
|
|
|
it('starts orphan reaping and reports failures without failing installation', async () => {
|
|
stateDirectory = await mkdtemp(join(tmpdir(), 'orca-structured-runtime-'))
|
|
const failure = new Error('scan failed')
|
|
const reapOrphanChildren = vi.fn(async () => {
|
|
throw failure
|
|
})
|
|
const onError = vi.fn()
|
|
|
|
await expect(
|
|
ensureStructuredAgentSessionHost({
|
|
stateDirectory,
|
|
hostId: HOST_ID,
|
|
claimKeyId: 'key-1',
|
|
resolveWorkspacePath: async () => stateDirectory!,
|
|
resolveClaudeAuthPolicy: () => ({ stripAuthEnv: true }),
|
|
resolveEnvironment: async () => ({}),
|
|
reapOrphanChildren,
|
|
onError
|
|
})
|
|
).resolves.toBeDefined()
|
|
|
|
await vi.waitFor(() =>
|
|
expect(onError).toHaveBeenCalledWith({
|
|
scope: 'agent-session-orphan-child-reaper',
|
|
error: failure
|
|
})
|
|
)
|
|
expect(reapOrphanChildren).toHaveBeenCalledWith({ store: expect.anything() })
|
|
})
|
|
|
|
it('logs an orphan-reaper failure when no reporter is configured', async () => {
|
|
stateDirectory = await mkdtemp(join(tmpdir(), 'orca-structured-runtime-'))
|
|
const failure = new Error('scan failed')
|
|
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
|
|
|
|
await expect(
|
|
ensureStructuredAgentSessionHost({
|
|
stateDirectory,
|
|
hostId: HOST_ID,
|
|
claimKeyId: 'key-1',
|
|
resolveWorkspacePath: async () => stateDirectory!,
|
|
resolveClaudeAuthPolicy: () => ({ stripAuthEnv: true }),
|
|
resolveEnvironment: async () => ({}),
|
|
reapOrphanChildren: async () => {
|
|
throw failure
|
|
}
|
|
})
|
|
).resolves.toBeDefined()
|
|
|
|
await vi.waitFor(() =>
|
|
expect(consoleError).toHaveBeenCalledWith(
|
|
'[structured-agent-session] orphan reaper failed',
|
|
failure
|
|
)
|
|
)
|
|
})
|
|
|
|
it('does not infer Windows process identity support from an injected reader', async () => {
|
|
stateDirectory = await mkdtemp(join(tmpdir(), 'orca-structured-runtime-'))
|
|
const originalPlatform = process.platform
|
|
const location: AgentSessionExecutionLocation = {
|
|
executionHostId: 'local',
|
|
wslDistro: null,
|
|
workspaceId: 'workspace-1',
|
|
workspaceKind: 'folder'
|
|
}
|
|
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
|
__setWindowsProcessTreeLoaderForTests(() => null)
|
|
try {
|
|
const host = await ensureStructuredAgentSessionHost({
|
|
stateDirectory,
|
|
hostId: HOST_ID,
|
|
claimKeyId: 'key-1',
|
|
resolveWorkspacePath: async () => stateDirectory!,
|
|
resolveEnvironment: async () => ({}),
|
|
resolveClaudeAuthPolicy: () => ({ stripAuthEnv: true }),
|
|
readProcessStartTime: async () => 1_700_000_000_000
|
|
})
|
|
|
|
expect(host.supportsCreate(location, 'codex')).toBe(false)
|
|
} finally {
|
|
__setWindowsProcessTreeLoaderForTests()
|
|
Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
|
|
}
|
|
})
|
|
})
|
|
|
|
// A stop whose teardown fails must not forget the runtime it was tearing down.
|
|
// `installing` is cleared either way so nothing new attaches, but the host keeps
|
|
// every journal whose close rejected, and this module slot is the only handle
|
|
// onto that host once it is gone.
|
|
describe('a teardown that fails is retried by the next stop', () => {
|
|
const JOURNAL_IDENTITY: AgentSessionJournalIdentity = {
|
|
sessionId: 'session-teardown-retry',
|
|
workspaceId: 'ws-1',
|
|
hostId: HOST_ID,
|
|
agent: 'codex',
|
|
providerHandle: { kind: 'codex', threadId: 'thread-1' }
|
|
}
|
|
const journals = createTrackedJournalOpener()
|
|
let directory: string | null = null
|
|
|
|
afterEach(async () => {
|
|
await agentSessionJournalCloseRetries.retryAll()
|
|
await journals.closeAll()
|
|
await stopStructuredAgentSessionRuntime().catch(() => undefined)
|
|
if (directory) {
|
|
await rm(directory, { recursive: true, force: true })
|
|
directory = null
|
|
}
|
|
})
|
|
|
|
it('reports the failure, then releases the handle on the following stop', async () => {
|
|
directory = await mkdtemp(join(tmpdir(), 'orca-structured-runtime-'))
|
|
await ensureStructuredAgentSessionHost({
|
|
stateDirectory: directory,
|
|
hostId: HOST_ID,
|
|
claimKeyId: 'key-1',
|
|
resolveWorkspacePath: async () => directory!,
|
|
resolveEnvironment: async () => ({}),
|
|
reapOrphanChildren: async () => [],
|
|
resolveClaudeAuthPolicy: () => ({ stripAuthEnv: true })
|
|
})
|
|
|
|
const journalDir = join(directory, 'stubborn-journal')
|
|
const real = await journals.open({ identity: JOURNAL_IDENTITY, journalDir })
|
|
let closeFailures = 2
|
|
const flaky = new Proxy(real, {
|
|
get(target, property, receiver) {
|
|
if (property !== 'close') {
|
|
// oxlint-disable-next-line anti-slop/no-reflect-get -- Proxy `get` trap: only Reflect.get forwards a raw string|symbol key with the proxy receiver.
|
|
return Reflect.get(target, property, receiver)
|
|
}
|
|
return async () => {
|
|
if (closeFailures > 0) {
|
|
closeFailures -= 1
|
|
throw new Error('close rejected')
|
|
}
|
|
await target.close()
|
|
}
|
|
}
|
|
})
|
|
await agentSessionJournalCloseRetries.closeOrRetain(flaky)
|
|
|
|
// The host's teardown runs the registry retry, so this stop surfaces it.
|
|
await expect(stopStructuredAgentSessionRuntime()).rejects.toThrow()
|
|
expect(agentSessionJournalCloseRetries.pendingDirectories).toEqual([journalDir])
|
|
|
|
// The retained runtime is what makes this a retry rather than a no-op.
|
|
await stopStructuredAgentSessionRuntime()
|
|
expect(agentSessionJournalCloseRetries.pendingDirectories).toEqual([])
|
|
})
|
|
})
|