Repository navigation
test(runtime): fail when Electron and orcad wire the runtime differently without a reason #30212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
343 changes: 343 additions & 0 deletions
343
src/main/runtime/construction/runtime-composition-parity.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,343 @@ | ||
| import { readFileSync } from 'node:fs' | ||
| import { join } from 'node:path' | ||
| import { describe, expect, it } from 'vitest' | ||
| import { RUNTIME_CAPABILITIES } from '../../../shared/protocol-version' | ||
| import { SSH_TARGET_MANAGEMENT_RUNTIME_CAPABILITY } from '../../../shared/ssh-target-management' | ||
| import { SKILL_INSTALL_CAPABILITY } from '../../../shared/skill-install-capability' | ||
|
|
||
| /** | ||
| * Electron (`main-process-*`) and orcad (`orcad-entry.ts`) each compose the same runtime, and every | ||
| * dependency is optional, so one root can silently skip a service the other wires (C2: #30120 never | ||
| * gave orcad its web client root). This records what each root's source wires and fails on any | ||
| * difference that is not declared below with a reason. | ||
| */ | ||
|
|
||
| type Root = 'electron' | 'orcad' | ||
|
|
||
| type DeclaredDifference = { | ||
| wiring: string | ||
| missingOn: Root | ||
| reason: string | ||
| /** Capabilities the missing host still advertises; its methods must refuse accurately (D3). */ | ||
| advertisedUnserved?: readonly string[] | ||
| } | ||
|
|
||
| const DECLARED_HOST_DIFFERENCES: readonly DeclaredDifference[] = [ | ||
| { | ||
| wiring: 'runtime.setArtifactService', | ||
| missingOn: 'orcad', | ||
| reason: 'Artifact sharing uses the desktop sign-in; a headless host has none yet.' | ||
| }, | ||
| { | ||
| wiring: 'runtime.setSkillCloudService', | ||
| missingOn: 'orcad', | ||
| reason: 'Skill sharing uses the desktop sign-in; installs from Git still work on orcad.', | ||
| advertisedUnserved: [SKILL_INSTALL_CAPABILITY] | ||
| }, | ||
| { | ||
| wiring: 'registerSshHandlers', | ||
| missingOn: 'orcad', | ||
| reason: 'orcad has no SSH stack; its SSH target methods refuse with an accurate message.', | ||
| advertisedUnserved: [SSH_TARGET_MANAGEMENT_RUNTIME_CAPABILITY] | ||
| }, | ||
| { | ||
| wiring: 'runtime.setAgentBrowserBridge', | ||
| missingOn: 'orcad', | ||
| reason: 'orcad installs browser automation through the runtime factory when Chromium exists.' | ||
| }, | ||
| { | ||
| wiring: 'runtime.setEmulatorBridge', | ||
| missingOn: 'orcad', | ||
| reason: 'Device emulators are desktop-only.' | ||
| }, | ||
| { | ||
| wiring: 'runtime.setOffscreenBrowserBackend', | ||
| missingOn: 'orcad', | ||
| reason: 'Offscreen WebContents need Electron.' | ||
| }, | ||
| { | ||
| wiring: 'ctor:prepareClaudeAuth', | ||
| missingOn: 'orcad', | ||
| reason: 'orcad passes it to registerHeadlessPtyRuntime instead, and only off WSL.' | ||
| }, | ||
| { | ||
| wiring: 'ctor:onPtyStopped', | ||
| missingOn: 'orcad', | ||
| reason: 'It clears renderer-side provider state, and orcad has no renderer.' | ||
| }, | ||
| { | ||
| wiring: 'ctor:onTerminalSideEffects', | ||
| missingOn: 'orcad', | ||
| reason: 'Side effects are delivered to a desktop window only.' | ||
| }, | ||
| { | ||
| wiring: 'ctor:attestAgentHookCompatibilityAuthority', | ||
| missingOn: 'orcad', | ||
| reason: 'Compatibility hook authority belongs to desktop-launched panes.' | ||
| }, | ||
| { | ||
| wiring: 'ctor:retireAgentHookCompatibilityAuthority', | ||
| missingOn: 'orcad', | ||
| reason: 'Pairs with attestAgentHookCompatibilityAuthority.' | ||
| }, | ||
| { | ||
| wiring: 'ctor:getPairedDeviceName', | ||
| missingOn: 'orcad', | ||
| reason: 'Unwired gap (follow-up): client-hosted browser rows show no paired-device name.' | ||
| }, | ||
| { | ||
| wiring: 'ctor:orchestrationEnvironmentTransport', | ||
| missingOn: 'orcad', | ||
| reason: 'Cross-environment orchestration dials paired environments from the desktop only.' | ||
| }, | ||
| { | ||
| wiring: 'ctor:skillTransactionRecovery', | ||
| missingOn: 'orcad', | ||
| reason: | ||
| 'No startup skill-transaction recovery runs on orcad, so installs need not wait for one.' | ||
| }, | ||
| { | ||
| wiring: 'rpc-option:exposeNetworkByDefault', | ||
| missingOn: 'orcad', | ||
| reason: 'orcad pins its bind host instead, so exposure never widens itself.' | ||
| }, | ||
| { | ||
| wiring: 'rpc.setMobileDesktopRelayHosts', | ||
| missingOn: 'orcad', | ||
| reason: 'The mobile desktop relay is hosted by the desktop.' | ||
| }, | ||
| { | ||
| wiring: 'rpc.setOnUnpairedDeviceAuthFailure', | ||
| missingOn: 'orcad', | ||
| reason: 'It notifies a desktop window.' | ||
| }, | ||
| { | ||
| wiring: 'rpc-option:pinnedBindHost', | ||
| missingOn: 'electron', | ||
| reason: 'The desktop binds loopback until a device pairs (STA-2370).' | ||
| } | ||
| ] | ||
|
|
||
| const ROOT_SOURCES: Record<Root, readonly string[]> = { | ||
| electron: [ | ||
| 'src/main/startup/main-process-runtime-service.ts', | ||
| 'src/main/startup/main-process-ready-runtime.ts', | ||
| 'src/main/startup/main-process-runtime-launch.ts', | ||
| 'src/main/window/attach-main-window-services.ts' | ||
| ], | ||
| orcad: ['src/main/orcad/orcad-entry.ts', 'src/main/orcad/orcad-account-services.ts'] | ||
| } | ||
|
|
||
| /** A spread with no inline object literal names the source of the object it spreads. */ | ||
| const SPREAD_SOURCES: Record<string, { file: string; anchor: string }> = { | ||
| 'agentHookRuntimeDeps(': { | ||
| file: 'src/main/agent-hooks/agent-hook-runtime-deps.ts', | ||
| anchor: 'return {' | ||
| }, | ||
| codexStructuredLaunchHomeResolvers: { | ||
| file: 'src/main/startup/codex-launch-preparation.ts', | ||
| anchor: 'codexStructuredLaunchHomeResolvers = {' | ||
| }, | ||
| 'accountLaunch?.runtimeDeps': { | ||
| file: 'src/main/orcad/orcad-account-services.ts', | ||
| anchor: 'runtimeDeps: {' | ||
| }, | ||
| 'orcadWsPortOptions(': { file: 'src/main/orcad/orcad-ws-port-policy.ts', anchor: 'wsPort: port' } | ||
| } | ||
|
|
||
| const repoRoot = join(__dirname, '../../../..') | ||
| const read = (file: string): string => stripComments(readFileSync(join(repoRoot, file), 'utf8')) | ||
|
|
||
| function stripComments(source: string): string { | ||
| let out = '' | ||
| let quote: string | null = null | ||
| for (let i = 0; i < source.length; i++) { | ||
| const char = source[i] | ||
| if (quote) { | ||
| out += char | ||
| if (char === '\\') { | ||
| out += source[++i] ?? '' | ||
| } else if (char === quote) { | ||
| quote = null | ||
| } | ||
| } else if (char === '/' && source[i + 1] === '/') { | ||
| while (i < source.length && source[i] !== '\n') { | ||
| i++ | ||
| } | ||
| out += '\n' | ||
| } else if (char === '/' && source[i + 1] === '*') { | ||
| i = source.indexOf('*/', i + 2) + 1 | ||
| } else { | ||
| if (char === "'" || char === '"' || char === '`') { | ||
| quote = char | ||
| } | ||
| out += char | ||
| } | ||
| } | ||
| return out | ||
| } | ||
|
|
||
| /** The inner text of the brace group that opens at `open` (strings are opaque). */ | ||
| function braceGroup(source: string, open: number): string { | ||
| let depth = 0 | ||
| let quote: string | null = null | ||
| for (let i = open; i < source.length; i++) { | ||
| const char = source[i] | ||
| if (quote) { | ||
| if (char === '\\') { | ||
| i++ | ||
| } else if (char === quote) { | ||
| quote = null | ||
| } | ||
| continue | ||
| } | ||
| if (char === "'" || char === '"' || char === '`') { | ||
| quote = char | ||
| } else if (char === '{') { | ||
| depth++ | ||
| } else if (char === '}' && --depth === 0) { | ||
| return source.slice(open + 1, i) | ||
| } | ||
| } | ||
| throw new Error('unbalanced braces') | ||
| } | ||
|
|
||
| /** The last match: a type declaring the same shape comes before the value. */ | ||
| function objectLiteralAfter(source: string, anchor: string): string { | ||
| const at = source.lastIndexOf(anchor) | ||
| if (at === -1) { | ||
| throw new Error(`anchor not found: ${anchor}`) | ||
| } | ||
| const anchorOpen = anchor.lastIndexOf('{') | ||
| // An anchor inside the literal (no brace of its own) belongs to the nearest enclosing `{`. | ||
| const open = anchorOpen !== -1 ? at + anchorOpen : source.lastIndexOf('{', at) | ||
| return braceGroup(source, open) | ||
| } | ||
|
|
||
| /** Top-level entries of an object literal's inner text, split on depth-0 commas. */ | ||
| function topLevelEntries(inner: string): string[] { | ||
| const entries: string[] = [] | ||
| let depth = 0 | ||
| let quote: string | null = null | ||
| let start = 0 | ||
| for (let i = 0; i < inner.length; i++) { | ||
| const char = inner[i] | ||
| if (quote) { | ||
| if (char === '\\') { | ||
| i++ | ||
| } else if (char === quote) { | ||
| quote = null | ||
| } | ||
| continue | ||
| } | ||
| if (char === "'" || char === '"' || char === '`') { | ||
| quote = char | ||
| } else if ('({['.includes(char)) { | ||
| depth++ | ||
| } else if (')}]'.includes(char)) { | ||
| depth-- | ||
| } else if (char === ',' && depth === 0) { | ||
| entries.push(inner.slice(start, i).trim()) | ||
| start = i + 1 | ||
| } | ||
| } | ||
| entries.push(inner.slice(start).trim()) | ||
| return entries.filter(Boolean) | ||
| } | ||
|
|
||
| function objectKeys(inner: string): string[] { | ||
| const keys: string[] = [] | ||
| for (const entry of topLevelEntries(inner)) { | ||
| if (!entry.startsWith('...')) { | ||
| const key = /^(?:async\s+)?([A-Za-z_$][\w$]*)/.exec(entry)?.[1] | ||
| if (!key) { | ||
| throw new Error(`unparsed object entry: ${entry}`) | ||
| } | ||
| keys.push(key) | ||
| continue | ||
| } | ||
| const spread = entry.slice(3).trim() | ||
| const source = Object.entries(SPREAD_SOURCES).find(([prefix]) => spread.startsWith(prefix)) | ||
| if (source) { | ||
| keys.push(...objectKeys(objectLiteralAfter(read(source[1].file), source[1].anchor))) | ||
| continue | ||
| } | ||
| // An inline conditional spread, e.g. `...(cond ? { a } : {})`: every literal it may spread. | ||
| let found = false | ||
| for (let i = spread.indexOf('{'); i !== -1; i = spread.indexOf('{', i + 1)) { | ||
| const group = braceGroup(spread, i) | ||
| keys.push(...objectKeys(group)) | ||
| i += group.length + 1 | ||
| found = true | ||
| } | ||
| if (!found) { | ||
| throw new Error(`teach SPREAD_SOURCES where this spread comes from: ${spread}`) | ||
| } | ||
| } | ||
| return keys | ||
| } | ||
|
|
||
| function recordWiring(root: Root): Set<string> { | ||
| const wired = new Set<string>() | ||
| for (const file of ROOT_SOURCES[root]) { | ||
| const source = read(file) | ||
| if (source.includes('new OrcaRuntimeService(')) { | ||
| const args = source.slice(source.indexOf('new OrcaRuntimeService(')) | ||
| for (const key of objectKeys(braceGroup(args, args.indexOf('{')))) { | ||
| wired.add(`ctor:${key}`) | ||
| } | ||
| } | ||
| if (source.includes('new OrcaRuntimeRpcServer(')) { | ||
| for (const key of objectKeys(objectLiteralAfter(source, 'new OrcaRuntimeRpcServer({'))) { | ||
| wired.add(`rpc-option:${key}`) | ||
| } | ||
| } | ||
| for (const [, receiver, setter] of source.matchAll( | ||
| /\b(runtime|runtimeRpc|rpc)\.(set[A-Z]\w*)\(/g | ||
| )) { | ||
| wired.add(`${receiver === 'runtime' ? 'runtime' : 'rpc'}.${setter}`) | ||
| } | ||
| if (/\bregister(?:HeadlessServe)?SshHandlers\(/.test(source)) { | ||
| wired.add('registerSshHandlers') | ||
| } | ||
| } | ||
| return wired | ||
| } | ||
|
|
||
| describe('runtime composition parity (Electron vs orcad)', () => { | ||
| const electron = recordWiring('electron') | ||
| const orcad = recordWiring('orcad') | ||
|
|
||
| it('records the wiring both roots share', () => { | ||
| for (const shared of [ | ||
| 'ctor:getLocalProvider', | ||
| 'ctor:onTerminalAgentStatus', | ||
| 'ctor:agentSessionClaimSigner', | ||
| 'rpc-option:webClientRoot', | ||
| 'rpc-option:wsPort', | ||
| 'runtime.setAccountServices' | ||
| ]) { | ||
| expect(electron, shared).toContain(shared) | ||
| expect(orcad, shared).toContain(shared) | ||
| } | ||
| }) | ||
|
|
||
| it('declares every wiring one root skips, and nothing it does not skip', () => { | ||
| const actual = [ | ||
| ...[...electron].filter((w) => !orcad.has(w)).map((w) => `orcad lacks ${w}`), | ||
| ...[...orcad].filter((w) => !electron.has(w)).map((w) => `electron lacks ${w}`) | ||
| ].sort() | ||
| const declared = DECLARED_HOST_DIFFERENCES.map((d) => `${d.missingOn} lacks ${d.wiring}`) | ||
| expect(actual).toEqual([...declared].sort()) | ||
| }) | ||
|
|
||
| it('lists each advertised-but-unserved capability against what the host advertises', () => { | ||
| for (const difference of DECLARED_HOST_DIFFERENCES) { | ||
| expect(difference.reason, difference.wiring).not.toBe('') | ||
| for (const capability of difference.advertisedUnserved ?? []) { | ||
| // Not advertised any more: drop it from the declaration. | ||
| expect(new Set<string>(RUNTIME_CAPABILITIES), difference.wiring).toContain(capability) | ||
| } | ||
| } | ||
| }) | ||
| }) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Fix the unterminated block comment handling in
stripComments.If
source.indexOf('*/', i + 2)returns-1, the expression-1 + 1setsito0. Theforloop then incrementsito1. The loop restarts from the file start and never ends. This only happens for a source file with an unterminated/*. The result is a hung test run instead of a clear error. The same loop can also repeat on a/*inside a regex literal.Throw an error when the terminator is missing.
Proposed fix
📝 Committable suggestion