-
-
Notifications
You must be signed in to change notification settings - Fork 315
refactor(apple): give snapshot and fold one native-build owner #3018
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,9 @@ import { readFile, rm, writeFile } from 'node:fs/promises'; | |
| import path from 'node:path'; | ||
| import { beforeAll, describe, test } from 'vitest'; | ||
| import { runCmd } from '@agent-device/host-kit/command'; | ||
| import { isRequestCanceledError } from '@agent-device/kernel/errors'; | ||
| import { mkdtempForTest } from '../__tests__/tmp-dir.ts'; | ||
| import { execKillTimeoutError } from '../snapshot-source/__tests__/exec-timeout-fixture.ts'; | ||
| import { execKillTimeoutError } from '../native-build/__tests__/exec-timeout-fixture.ts'; | ||
| import { createSnapshotSourceHost } from '../snapshot-source/host.ts'; | ||
| import type { SnapshotSourceHost } from '../snapshot-source/types.ts'; | ||
| import { | ||
|
|
@@ -156,6 +157,64 @@ test('a compile exec killed at its budget reports the fold-helper build, not the | |
| } | ||
| }); | ||
|
|
||
| test('an abort while a second caller waits on the fold-helper lock reports a canceled request', async () => { | ||
| const root = await mkdtempForTest('agent-device-fold-helper-cache-lock-wait-'); | ||
| const sourceRoot = path.join(root, 'source'); | ||
| const cacheRoot = path.join(root, 'cache'); | ||
| await (await import('@agent-device/host-kit/host-file')).ensureHostDirectory(sourceRoot); | ||
| await writeFile(path.join(sourceRoot, 'Fold.m'), 'fold source'); | ||
|
|
||
| let releaseClang!: () => void; | ||
| const clangGate = new Promise<void>((resolve) => { | ||
| releaseClang = resolve; | ||
| }); | ||
| let clangStarted!: () => void; | ||
| const clangStartedSignal = new Promise<void>((resolve) => { | ||
| clangStarted = resolve; | ||
| }); | ||
| const host = fakeFoldHelperHost(() => 'binary'); | ||
| const holdingHost: SnapshotSourceHost = { | ||
| ...host, | ||
| run: async (command, args, options) => { | ||
| if (command === 'xcrun' && args.includes('clang')) { | ||
| clangStarted(); | ||
| await clangGate; | ||
| return await host.run(command, args, options); | ||
| } | ||
| return await host.run(command, args, options); | ||
| }, | ||
| }; | ||
|
|
||
| try { | ||
| // The first call acquires the fold-helper lock and holds it in its build step (gated on | ||
| // `clangGate`) until this test releases it, so the second call below is guaranteed to find | ||
| // the lock already held rather than racing for it. | ||
| const holder = ensureFoldHelperBinary({ host: holdingHost, sourceRoot, cacheRoot }); | ||
| await clangStartedSignal; | ||
|
|
||
| const controller = new AbortController(); | ||
| const waiter = ensureFoldHelperBinary({ | ||
| host, | ||
| sourceRoot, | ||
| cacheRoot, | ||
| signal: controller.signal, | ||
| }); | ||
| // Give the waiter time to reach the lock's poll loop before aborting it. | ||
| await new Promise((resolve) => setTimeout(resolve, 50)); | ||
| controller.abort(); | ||
|
|
||
| await assert.rejects(waiter, (error: unknown) => { | ||
| assert.ok(isRequestCanceledError(error), 'expected a canceled-request error'); | ||
| return true; | ||
| }); | ||
|
|
||
| releaseClang(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Prompt for AI agents |
||
| await holder; | ||
| } finally { | ||
| await rm(root, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| // #2796: the production compile drops -Werror so a stale toolchain warning cannot fail a build; | ||
| // this is the gate that keeps a new Fold.m warning from passing CI unnoticed. It runs the | ||
| // production argv (`buildFoldHelperCompileArgv`) against the real iphonesimulator SDK with | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,19 @@ | ||
| import path from 'node:path'; | ||
| import { AppError } from '@agent-device/kernel/errors'; | ||
| import { execFailureDetails } from '@agent-device/host-kit/command'; | ||
| import { hostHomeDirectory } from '@agent-device/host-kit/host-file'; | ||
| import { findProjectRoot } from '@agent-device/host-kit/version'; | ||
| import { runAppleToolCommand } from '../core/tool-provider.ts'; | ||
| import { COLD_TOOLCHAIN_PROBE_TIMEOUT_MS } from '../runner/apple-runner-platform.ts'; | ||
| import { readHostToolchainIdentity } from '../snapshot-source/cache-identity.ts'; | ||
| import { | ||
| createSnapshotSourceDeadline, | ||
| type SnapshotSourceDeadline, | ||
| } from '../snapshot-source/deadline.ts'; | ||
| import { SnapshotSourceError } from '../snapshot-source/errors.ts'; | ||
| import { createSnapshotSourceHost } from '../snapshot-source/host.ts'; | ||
| import { createNativeBuildDeadline, type NativeBuildDeadline } from '../native-build/deadline.ts'; | ||
| import { NativeBuildError } from '../native-build/errors.ts'; | ||
| import { createNativeBuildHost, type NativeBuildHost } from '../native-build/host.ts'; | ||
| import { readHostToolchainIdentity } from '../native-build/toolchain-identity.ts'; | ||
| import { | ||
| ensureNativeBuildCacheEntry, | ||
| execNativeBuildClang, | ||
| fingerprintNativeBuildSource, | ||
| } from '../snapshot-source/native-build-cache.ts'; | ||
| import type { SnapshotSourceHost } from '../snapshot-source/types.ts'; | ||
| } from '../native-build/cache.ts'; | ||
|
|
||
| const FOLD_HELPER_SOURCE_FILENAME = 'Fold.m'; | ||
| const FOLD_HELPER_BINARY_FILENAME = 'fold-helper'; | ||
|
|
@@ -33,9 +31,9 @@ const FOLD_HELPER_PREPARATION_DEADLINE_MS = | |
|
|
||
| /** | ||
| * The fold helper binary for the host's active toolchain, building and caching it if needed. Shares | ||
| * the snapshot bridge's content+toolchain-keyed build cache (`native-build-cache.ts`), so a fold | ||
| * the snapshot bridge's content+toolchain-keyed build cache (`native-build/cache.ts`), so a fold | ||
| * call after the first serves a cached binary instead of recompiling `Fold.m`, and a `DEVELOPER_DIR` | ||
| * switch busts the cache instead of serving a binary built against a different SDK (#2796). | ||
| * switch busts the cache instead of serving a binary built against a different SDK (#2796, #2970). | ||
| * | ||
| * Build and cache failures surface as `AppError('COMMAND_FAILED', ..., {reason: | ||
| * 'fold-helper-build-failed'})`, the error shape `sendSimulatorFoldPose` reported before this cache | ||
|
|
@@ -44,15 +42,15 @@ const FOLD_HELPER_PREPARATION_DEADLINE_MS = | |
| export async function ensureFoldHelperBinary( | ||
| input: Readonly<{ | ||
| signal?: AbortSignal; | ||
| host?: SnapshotSourceHost; | ||
| host?: NativeBuildHost; | ||
| cacheRoot?: string; | ||
| sourceRoot?: string; | ||
| }> = {}, | ||
| ): Promise<Readonly<{ path: string }>> { | ||
| const host = input.host ?? createFoldHelperCacheHost(); | ||
| const deadline = createSnapshotSourceDeadline(FOLD_HELPER_PREPARATION_DEADLINE_MS, input.signal); | ||
| const deadline = createNativeBuildDeadline(FOLD_HELPER_PREPARATION_DEADLINE_MS, input.signal); | ||
| try { | ||
| const sourceRoot = input.sourceRoot ?? path.join(host.projectRoot(), 'apple', 'fold-helper'); | ||
| const sourceRoot = input.sourceRoot ?? path.join(findProjectRoot(), 'apple', 'fold-helper'); | ||
| const sourceHash = await fingerprintNativeBuildSource( | ||
| host, | ||
| sourceRoot, | ||
|
|
@@ -61,7 +59,7 @@ export async function ensureFoldHelperBinary( | |
| ); | ||
| const toolchain = await readHostToolchainIdentity(host, deadline); | ||
| const cacheRoot = | ||
| input.cacheRoot ?? path.join(host.homeDirectory(), '.agent-device', 'fold-helper'); | ||
| input.cacheRoot ?? path.join(hostHomeDirectory(), '.agent-device', 'fold-helper'); | ||
| return await ensureNativeBuildCacheEntry({ | ||
| host, | ||
| deadline, | ||
|
|
@@ -82,14 +80,12 @@ export async function ensureFoldHelperBinary( | |
| } | ||
| } | ||
|
|
||
| function createFoldHelperCacheHost(): SnapshotSourceHost { | ||
| const real = createSnapshotSourceHost(); | ||
| return { | ||
| ...real, | ||
| // Routed through the Apple tool-provider scope, not `run`'s default `runCmd`, so a fold test | ||
| // can fake every exec this cache makes the same way it fakes the simctl dispatch (#2796). | ||
| run: (command, args, options) => runAppleToolCommand(command, args, options), | ||
| }; | ||
| function createFoldHelperCacheHost(): NativeBuildHost { | ||
| // Routed through the Apple tool-provider scope, not `run`'s default `runCmd`, so a fold test can | ||
| // fake every exec this cache makes the same way it fakes the simctl dispatch (#2796). A narrow | ||
| // build host, not the full snapshot-bridge host: compilation needs no bridge socket and no | ||
| // target-process inspection (#2970). | ||
| return createNativeBuildHost(runAppleToolCommand); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -119,8 +115,8 @@ export function buildFoldHelperCompileArgv( | |
| } | ||
|
|
||
| async function compileFoldHelper( | ||
| host: SnapshotSourceHost, | ||
| deadline: SnapshotSourceDeadline, | ||
| host: NativeBuildHost, | ||
| deadline: NativeBuildDeadline, | ||
| sourceRoot: string, | ||
| outputPath: string, | ||
| ): Promise<void> { | ||
|
|
@@ -137,13 +133,13 @@ async function compileFoldHelper( | |
| } | ||
|
|
||
| /** | ||
| * Rewraps a cache failure as the fold helper's build error, keeping its hint and typed details; a | ||
| * cancellation, and any error that is not a snapshot-source failure, passes through unchanged. | ||
| * Rewraps a native-build cache failure as the fold helper's build error, keeping its hint and typed | ||
| * details; a cancellation, and any error that is not a native-build failure (including the fold | ||
| * helper's own `foldHelperBuildFailed`, already in its public shape), passes through unchanged. | ||
| */ | ||
| function asFoldHelperCacheError(error: unknown): unknown { | ||
| if (!(error instanceof SnapshotSourceError) || error.failureKind === 'cancelled') return error; | ||
| const { bridgeFailure: _kind, bridgeFailureCode: cause, ...details } = error.details ?? {}; | ||
| return foldHelperBuildFailed({ ...details, cause }, error); | ||
| if (!(error instanceof NativeBuildError) || error.buildFailureKind === 'cancelled') return error; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Abort during fold-helper preparation now returns Prompt for AI agents |
||
| return foldHelperBuildFailed({ ...error.buildDetails, cause: error.buildFailureCode }, error); | ||
| } | ||
|
|
||
| function foldHelperBuildFailed(details: Readonly<Record<string, unknown>>, cause?: unknown) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
P3: The 50 ms sleep is the only mechanism aiming the abort at the lock-wait branch, and the test still passes when it misses: if the abort lands before the waiter reaches
acquireLock,remainingNativeBuildMsor the aborted-signal checks increateNativeBuildDeadline/acquireNativeBuildLockreject withNativeBuildError('cancelled', 'abort-signal'), soisRequestCanceledErrorholds without exercising the lock-wait path this test is named for. That silently weakens the regression coverage the fix (lock-wait abort keepingreason: 'request_canceled') needs. Signal from the waiter's lock acquisition instead of sleeping: wraphost.acquireLockin the waiter's host so it resolves alockWaitStartedpromise, thenawait lockWaitStartedbeforecontroller.abort(). The outcome is not flaky either way: every pre-lock abort path also yields a cancelled error, so the assertion itself is deterministic.Prompt for AI agents