mirror of
https://github.com/stablyai/orca.git
synced 2026-10-06 16:02:25 +00:00
test: retire src/main cases that replay a contract their owner already proves (#24025)
Audit sweep over `src/main/{native-chat,startup,daemon,skills,ssh,providers,git,
persistence,claude,agent-hooks,github}`. 35 case declarations removed across 23
files, 1 test file deleted, 655 lines gone. No production file touched.
What went, by pattern:
- Duplicate invocations of a contract owned exhaustively elsewhere: three
`publishDaemonEndpoint` cases that `daemon-endpoint-publish.test.ts` already
covers in 20, and three daemon health classifications (`HEALTHY`, `DEGRADED`,
`UNREACHABLE`) that `daemon-health.test.ts` owns. `WEDGED` and `WEDGED-HELLO`
stayed — the never-resolving-RPC and never-answers-hello paths have no other
owner.
- Provider-local replays of a shared helper: five `GitStatusReadLeaseOwner` cases
re-run per provider, owned by `src/main/git/git-status-read-lease-owner.test.ts`,
and `returns the connectionId` replayed in three provider suites against an
identity getter.
- Assertion-free coverage probes, including one whose comment says "no writes
should happen" while nothing checks that.
- Copied inventories that restate a type: `PROVIDER_FRAME_CLASSIFICATIONS` is
declared `as const satisfies Record<...>`, so a missing key is already a type
error and an extra key fails the excess-property check. Those cases also pinned
key order, which is not a contract.
- A negative control that cannot fail: asserting a profile-state filename is not
an unrelated literal, in a file whose first case already pins that filename
positively.
- Byte-identical duplicates across files, and a second case re-asserting the
`unverifiable -> true` mapping the case above it already proves.
`src/main/providers/ssh-git-provider-api.test.ts` goes: 52 method names asserted
`toBeTypeOf('function')` plus `toHaveLength(52)` over its own literal. Note the
reason, because the obvious one is wrong. "The `IGitProvider & SshGitProvider`
annotation enforces this at compile time" does NOT hold — removing an operation
from the interface and its implementing class in one commit still compiles. What
makes the file redundant is that all 51 extractable names are referenced by some
other test under `src`, so dropping an operation breaks a behavioral test anyway.
The same check kept the three `registers all expected handlers` manifests in
`src/relay` during the previous wave, where eleven methods had no behavioral
caller at all. An inventory test is a ratchet if and only if at least one entry is
pinned solely by it; that is verified per entry, not per file.
Kept deliberately: everything a reliability gate cites, checked by case TITLE and
not only by file path, because the gate script resolves paths only; bound, quota
and provenance guards; the Windows MSYS job-breakaway and daemon-host relocation
tests, which guard failures that pass every existing gate; SSH execution-boundary
verdict vocabulary; and Git capability tests covering first fallback, cached call,
concurrent probes and per-host isolation as four distinct risks.
Coverage is partial and stated as such: of 1,449 files in scope, roughly 990 were
read case-by-case and 452 received title-and-grep triage only. The unread paths
are recorded for a later sweep rather than assumed clean.
Verified: per-area suites green (`daemon`+`skills` 294 files/3016 cases;
`git`+`persistence` 414 files/4476 cases; and the rest), gate manifest 140 gates,
`check:code-quality:changed` 0 new findings. A combined 11-path local run put
1,563 files through one machine and surfaced three timing-sensitive failures in
files this change does not touch (`history-manager`,
`structured-agent-session-refusal-retry`, `ssh-remote-commands`); all three pass
in isolation, and no production code changed, so CI's sharded run is the arbiter.
This commit is contained in:
Neil
@@ -555,12 +555,6 @@ describe('SshChannelMultiplexer', () => {
|
||||
expect(disposeHandler).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('ignores notify after dispose', () => {
|
||||
mux.dispose()
|
||||
mux.notify('pty.data', { id: 'pty-1', data: 'x' })
|
||||
// No writes should happen after the initial keepalive writes
|
||||
})
|
||||
|
||||
it('reports isDisposed correctly', () => {
|
||||
expect(mux.isDisposed()).toBe(false)
|
||||
mux.dispose()
|
||||
|
||||
@@ -57,11 +57,6 @@ describe('SshConnectionManager', () => {
|
||||
expect(mgr.getConnection(target.id)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('disconnect is a no-op for unknown targets', async () => {
|
||||
const mgr = new SshConnectionManager(createCallbacks())
|
||||
await mgr.disconnect('unknown')
|
||||
})
|
||||
|
||||
it('reuses existing connected connection for same target', async () => {
|
||||
const mgr = new SshConnectionManager(createCallbacks())
|
||||
const target = createTarget()
|
||||
|
||||
@@ -1,27 +1,9 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
assertSafeRemotePathSegment,
|
||||
getRemoteHostPlatform,
|
||||
joinRemotePath
|
||||
} from './ssh-remote-platform'
|
||||
import { detectRemoteHostPlatform } from './ssh-remote-platform-detection'
|
||||
import { execCommand } from './ssh-relay-deploy-helpers'
|
||||
import type { SshConnection } from './ssh-connection'
|
||||
|
||||
vi.mock('./ssh-relay-deploy-helpers', () => ({
|
||||
execCommand: vi.fn()
|
||||
}))
|
||||
|
||||
const conn = {} as SshConnection
|
||||
|
||||
function decodePowerShellCommand(command: string): string {
|
||||
const match = command.match(/-EncodedCommand\s+([A-Za-z0-9+/=]+)/)
|
||||
return match ? Buffer.from(match[1], 'base64').toString('utf16le') : ''
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
describe('joinRemotePath', () => {
|
||||
it('joins POSIX remote paths', () => {
|
||||
@@ -77,33 +59,3 @@ describe('assertSafeRemotePathSegment', () => {
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('detectRemoteHostPlatform', () => {
|
||||
it('uses uname when the remote is POSIX', async () => {
|
||||
vi.mocked(execCommand).mockResolvedValueOnce('__ORCA_REMOTE_PLATFORM__ Darwin arm64')
|
||||
|
||||
await expect(detectRemoteHostPlatform(conn)).resolves.toMatchObject({
|
||||
relayPlatform: 'darwin-arm64',
|
||||
commandDialect: 'posix'
|
||||
})
|
||||
})
|
||||
|
||||
it('falls back to PowerShell when uname is unavailable on Windows', async () => {
|
||||
vi.mocked(execCommand)
|
||||
.mockRejectedValueOnce(new Error('uname not recognized'))
|
||||
.mockResolvedValueOnce('__ORCA_REMOTE_PLATFORM__ Windows AMD64')
|
||||
|
||||
await expect(detectRemoteHostPlatform(conn)).resolves.toMatchObject({
|
||||
relayPlatform: 'win32-x64',
|
||||
commandDialect: 'powershell',
|
||||
pathFlavor: 'windows'
|
||||
})
|
||||
|
||||
expect(vi.mocked(execCommand).mock.calls[1]?.[1]).toContain('powershell.exe')
|
||||
const script = decodePowerShellCommand(vi.mocked(execCommand).mock.calls[1]?.[1] ?? '')
|
||||
expect(script).toContain('$arch = $env:PROCESSOR_ARCHITECTURE')
|
||||
expect(script).toContain('try { $runtimeArch =')
|
||||
expect(script).toContain('catch {}')
|
||||
expect(script).toContain('Write-Output ("`n__ORCA_REMOTE_PLATFORM__ Windows " + $arch)')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -32,7 +32,6 @@ import {
|
||||
writeBufferViaSystemSsh,
|
||||
writeFileViaSystemSsh
|
||||
} from './ssh-system-fallback'
|
||||
import { spawnSystemSshPortForward } from './system-ssh-forward-process'
|
||||
import { getRemoteHostPlatform } from './ssh-remote-platform'
|
||||
import type { SshTarget } from '../../shared/ssh-types'
|
||||
import type { SystemSshResolvedConfig } from './ssh-control-socket'
|
||||
@@ -506,36 +505,6 @@ describe('spawnSystemSsh', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('spawns port forwards before the ssh destination terminator', () => {
|
||||
spawnSystemSshPortForward(createTarget({ configHost: 'fdpass-host' }), 5173, '127.0.0.1', 3000)
|
||||
|
||||
const args = spawnMock.mock.calls[0][1] as string[]
|
||||
const terminatorIdx = args.indexOf('--')
|
||||
const forwardFlagIdx = args.indexOf('-N')
|
||||
const localForwardIdx = args.indexOf('-L')
|
||||
const exitOnForwardFailureIdx = args.indexOf('ExitOnForwardFailure=yes')
|
||||
const standaloneControlIdx = args.indexOf('-S')
|
||||
|
||||
expect(terminatorIdx).toBeGreaterThan(-1)
|
||||
expect(forwardFlagIdx).toBeGreaterThan(-1)
|
||||
expect(localForwardIdx).toBeGreaterThan(-1)
|
||||
expect(exitOnForwardFailureIdx).toBeGreaterThan(-1)
|
||||
// Why: -N and -L must appear before -- or OpenSSH treats them as remote command args.
|
||||
expect(forwardFlagIdx).toBeLessThan(terminatorIdx)
|
||||
expect(localForwardIdx).toBeLessThan(terminatorIdx)
|
||||
expect(args[exitOnForwardFailureIdx - 1]).toBe('-o')
|
||||
expect(exitOnForwardFailureIdx).toBeLessThan(terminatorIdx)
|
||||
expect(standaloneControlIdx).toBe(-1)
|
||||
expectNoOrcaControlMasterArgs(args)
|
||||
expect(args).toContain('127.0.0.1:5173:127.0.0.1:3000')
|
||||
expect(args[terminatorIdx + 1]).toBe('fdpass-host')
|
||||
expect(spawnMock).toHaveBeenCalledWith(
|
||||
SYSTEM_SSH_PATH,
|
||||
expect.any(Array),
|
||||
expect.objectContaining({ stdio: ['ignore', 'ignore', 'pipe'] })
|
||||
)
|
||||
})
|
||||
|
||||
it('can spawn a native remote command without the POSIX shell wrapper', () => {
|
||||
spawnSystemSshCommand(createTarget({ configHost: 'fdpass-host' }), 'echo hello', {
|
||||
wrapCommand: false
|
||||
|
||||
Reference in New Issue
Block a user