Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions .fallowrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -567,15 +567,7 @@
"exports": ["CommandPolicy", "CommandSessionRecord"]
}
],
"usedClassMembers": [
"name",
"listActiveLeases",
"delete",
"values",
"elapsedMs",
"isExpired",
"findByDevice"
],
"usedClassMembers": ["name", "listActiveLeases", "delete", "values", "elapsedMs", "isExpired"],
"rules": {
"unused-types": "error",
"duplicate-exports": "off"
Expand Down
5 changes: 3 additions & 2 deletions docs/adr/0007-remote-device-leases.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,9 @@ app for every request but `open` and the `batch` envelope, so a
request naming no session cannot fall back to the host Mac. `open` requires the native app backend (ADR 0031), because XCTest
posts screen events that can land outside the app's window. A pid-pinned lease is checked against the
running process before each admitted request. A session opened under the lease holds its app, not
the Mac: it takes no host device claim, and other app-leased sessions on the same Mac do not conflict
with it, so one daemon serves several leased apps beside the host's own sessions.
the Mac: it takes no host device claim, and sessions on other apps of the same Mac, leased or
holding an app claim (ADR 0034), do not conflict with it, so one daemon serves several leased apps
beside the host's own sessions. A session on the same app does.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The new wording says leased apps run beside the host's own sessions, but a whole-Mac session in the same daemon now blocks a leased open. The new-session check runs sessionConflictsWithOpen, which treats a whole-device session as conflicting with any app opener. Name the exception so the ADR matches ADR 0034 rule 5.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0007-remote-device-leases.md, line 194:

<comment>The new wording says leased apps run beside the host's own sessions, but a whole-Mac session in the same daemon now blocks a leased open. The new-session check runs `sessionConflictsWithOpen`, which treats a whole-device session as conflicting with any app opener. Name the exception so the ADR matches ADR 0034 rule 5.</comment>

<file context>
@@ -189,8 +189,9 @@ app for every request but `open` and the `batch` envelope, so a
-with it, so one daemon serves several leased apps beside the host's own sessions.
+the Mac: it takes no host device claim, and sessions on other apps of the same Mac, leased or
+holding an app claim (ADR 0034), do not conflict with it, so one daemon serves several leased apps
+beside the host's own sessions. A session on the same app does.
 
 These rules bind a request that names the lease or runs in its session. A daemon policy
</file context>
Suggested change
beside the host's own sessions. A session on the same app does.
beside the host's app-claimed sessions on other apps. A whole-Mac session in the same daemon, or a session on the same app, does conflict.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This overstates same-app exclusion across daemons: a macos-app lease takes no shared host claim, so another daemon's app claim can open the same bundle ID without seeing the leased session. Scope this guarantee to sessions in one daemon.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0007-remote-device-leases.md, line 194:

<comment>This overstates same-app exclusion across daemons: a `macos-app` lease takes no shared host claim, so another daemon's app claim can open the same bundle ID without seeing the leased session. Scope this guarantee to sessions in one daemon.</comment>

<file context>
@@ -189,8 +189,9 @@ app for every request but `open` and the `batch` envelope, so a
-with it, so one daemon serves several leased apps beside the host's own sessions.
+the Mac: it takes no host device claim, and sessions on other apps of the same Mac, leased or
+holding an app claim (ADR 0034), do not conflict with it, so one daemon serves several leased apps
+beside the host's own sessions. A session on the same app does.
 
 These rules bind a request that names the lease or runs in its session. A daemon policy
</file context>
Suggested change
beside the host's own sessions. A session on the same app does.
beside the host's own sessions. In one daemon, a session on the same app does.


These rules bind a request that names the lease or runs in its session. A daemon policy
`leases.require` (ADR 0029) refuses requests that name no lease. The proxy token is one credential
Expand Down
2 changes: 2 additions & 0 deletions docs/adr/0031-macos-native-app-backend.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ window fields are unmeasured.
assistive client. The app surface walks up to 48 levels deep and reports a deeper tree as
truncated; other helper surfaces keep 12.
6. Screenshots capture the session app's front window by itself through ScreenCaptureKit.
7. An app session claims only its app, and `open` launches or reopens that app with `open -g`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This rule promises per-app claims and background open -g for every app session, but those behaviors apply only to native sessions holding a per-app claim; unresolved targets and XCTest sessions retain whole-device behavior. Scope the rule to sessions holding a per-app claim.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0031-macos-native-app-backend.md, line 39:

<comment>This rule promises per-app claims and background `open -g` for every app session, but those behaviors apply only to native sessions holding a per-app claim; unresolved targets and XCTest sessions retain whole-device behavior. Scope the rule to sessions holding a per-app claim.</comment>

<file context>
@@ -36,6 +36,8 @@ window fields are unmeasured.
    assistive client. The app surface walks up to 48 levels deep and reports a deeper tree as
    truncated; other helper surfaces keep 12.
 6. Screenshots capture the session app's front window by itself through ScreenCaptureKit.
+7. An app session claims only its app, and `open` launches or reopens that app with `open -g`,
+   leaving the frontmost app in front ([ADR 0034](0034-macos-per-app-claims.md)).
 
</file context>
Suggested change
7. An app session claims only its app, and `open` launches or reopens that app with `open -g`,
7. A native macOS session holding a per-app claim claims only its app, and `open` launches or reopens that app with `open -g`,

leaving the frontmost app in front ([ADR 0034](0034-macos-per-app-claims.md)).

## Context

Expand Down
83 changes: 83 additions & 0 deletions docs/adr/0034-macos-per-app-claims.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# ADR 0034: macOS Per-App Device Claims

## Status

Accepted (2026-10-08). Applies to the native macOS app backend (ADR 0031) only. Numbered 0034

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The numbering note is process history, which the ADR README says belongs in git history. Drop the 'Numbered 0034 because 0032 and 0033 exist on main.' sentence from Status.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0034-macos-per-app-claims.md, line 5:

<comment>The numbering note is process history, which the ADR README says belongs in git history. Drop the 'Numbered 0034 because 0032 and 0033 exist on main.' sentence from Status.</comment>

<file context>
@@ -0,0 +1,83 @@
+
+## Status
+
+Accepted (2026-10-08). Applies to the native macOS app backend (ADR 0031) only. Numbered 0034
+because 0032 and 0033 exist on main.
+
</file context>
Suggested change
Accepted (2026-10-08). Applies to the native macOS app backend (ADR 0031) only. Numbered 0034
Accepted (2026-10-08). Applies to the native macOS app backend (ADR 0031) only.

because 0032 and 0033 exist on main.

## Rules at a glance

1. A native macOS `app` session whose app resolves to a bundle id before launch claims
`<device key>:app:<bundleId>` instead of the device key. Every other macOS session (XCTest,
`desktop`, `menubar`, `frontmost-app`, an open of a URL alone) claims the device key. A
`macos-app` lease still takes no claim (ADR 0007).
2. The per-app record is the process-owned v2 record with `app: { bundleId }`; it decodes only
at the key derived from its identity and app. Classification, takeover, close, idle expiry,
shutdown, the startup sweep, and `device release --stale` treat it like any claim.
3. Lock order is device key, then app key. Every acquisition on a device holds the device key's
lock; a per-app acquisition takes the app key's lock inside it. Renew, clear, abandon, the
sweep, and stale release take only their own key's lock and never create a claim.
4. A per-app acquisition settles the device key's claim first: a live foreign whole-device claim
is a conflict. A whole-device acquisition on a macOS device scans the per-app claims of that
device: dead or superseded owners are reconciled and removed, other foreign claims conflict,
and this daemon's own are left to its session store.
5. In one daemon, a session holds either one app (an app claim, or a `macos-app` lease, which
takes no claim) or the whole device. Two sessions on the device conflict unless each holds one
app and the apps differ. The new-session conflict check and the `open --wait` that queues on it
decide with this one predicate, so a whole-device opener and a same-app opener wait for an app
session and a different-app opener runs beside it. A per-app session may reopen only its app on
the `app` surface; a link opens as `open <app> <url>`, because a bare URL's handler is not known
before launch.
6. Helper screenshots take a host-wide lock (`macos-screen-capture.lock` in the device claims
directory) for the length of one capture. It is the only action sessions on different apps
serialize on.
7. App keys are lowercase, and the predicate of rule 5 compares bundle ids without regard to case,
because LaunchServices matches them that way. An `open --wait` resolves the claim it would take
under the device lock; until then it waits only for a whole-device session, and when the open
then finds a conflicting app session it releases the lock and waits for that session too.

## Context

The Mac is one device, so its one claim let a single session per Mac drive any macOS app. The
native backend acts on the session app alone: accessibility actions on its elements, key events
posted to its process, window capture, and (rule 7 of ADR 0031) a background `open`. None of
those moves the real pointer or changes the frontmost app, so two sessions on two apps cannot
interfere through focus or the pointer. One host resource is shared: ScreenCaptureKit. Two helper
processes capturing two app windows at the same moment failed (`screenshot failed`, or both hung
until the 30 s helper timeout) in each of four attempts on macOS 27, while each capture alone took
about 0.5 s, so captures run one at a time.

## Decision details

**Bundle id, not pid.** The helper resolves its target app by bundle id, so two processes of one
bundle id are one target to every action. A pid in the key would let two sessions drive the same
windows. Liveness is the owning daemon's process, as for every claim. Two copies of one app need
two bundle ids.

**Key resolved before the claim.** The bundle id comes from the read-only open-target resolution,
before the claim and before launch. An open whose app cannot be resolved that early takes the
whole device rather than guessing.

**What stays whole-Mac.** Surfaces other than `app` post real pointer events or read every app.
Some commands still reach past the session app, as they did before this ADR: the clipboard and
appearance settings (host-global state), `close <other app>` (quits any app, including one another
daemon's session holds), and `settings permission` (opens System Settings in front).

**Background open follows the session's scope.** A session that holds one app opens it with
`open -g`, because bringing it forward would move the frontmost app under every other session.
A session that holds the whole Mac, including `open <app> --surface frontmost-app`, opens the app in
front as before.

## Rejected alternatives

- **An input lock around every action.** The native actions need neither focus nor the pointer
once `open` stops activating, so a lock would only serialize sessions for nothing. Only the
capture, which contends on ScreenCaptureKit rather than on input, is serialized.
- **A separate claims directory per app.** Two stores could not exclude each other under one
lock, and every stale-claim path would need a second scan.

## Support boundary

A daemon older than this ADR reads a per-app record as inconsistent and never clears it, and its

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The support boundary should state the consequence: a whole-Mac session from an older daemon can run beside an app session, and two sessions can drive the same app. Name that risk so operators know what 'one version per claims directory' prevents.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0034-macos-per-app-claims.md, line 81:

<comment>The support boundary should state the consequence: a whole-Mac session from an older daemon can run beside an app session, and two sessions can drive the same app. Name that risk so operators know what 'one version per claims directory' prevents.</comment>

<file context>
@@ -0,0 +1,83 @@
+
+## Support boundary
+
+A daemon older than this ADR reads a per-app record as inconsistent and never clears it, and its
+whole-device acquisition does not scan per-app records. Run one version per claims directory, as
+ADR 0030 requires for process locks.
</file context>

whole-device acquisition does not scan per-app records. Run one version per claims directory, as
ADR 0030 requires for process locks.
1 change: 1 addition & 0 deletions docs/adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
| [0031 macOS Native App Backend](0031-macos-native-app-backend.md) | `AGENT_DEVICE_MACOS_APP_BACKEND`, driving macOS app sessions without XCTest Automation Mode, why pointer actions are accessibility actions only, and the ghost cursor |
| [0032 Layering Graph Engine](0032-layering-graph-engine.md) | evaluating replacement of `scripts/layering/` graph rules with dependency-cruiser or fallow `boundaries`, why graph-based layering checks share one replaceable import model, the measured edge-set and parity gaps, and the revisit triggers |
| [0033 Collocation decision table](0033-collocation-decision-table.md) | moving or adding a file listed in umbrella #3276 §5, re-proposing a move that table refused, reading a zone's collocation decisions |
| [0034 macOS Per-App Device Claims](0034-macos-per-app-claims.md) | claim keys below a device, the device-then-app lock order, native macOS app sessions running beside each other, stale per-app claims |

ADRs record *why*; the registries and gates they describe are the living source of truth — when
prose and a registry disagree, the registry wins and the ADR needs a follow-up.
Expand Down
4 changes: 4 additions & 0 deletions packages/contracts/src/application-lifecycle-interaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ export async function invokeApplicationOpen(
appBundleId?: string;
execution: ApplicationLifecycleExecution;
terminateRunningApp?: boolean;
background?: boolean;
}>,
): Promise<void> {
const { device, positionals, execution } = params;
Expand All @@ -166,6 +167,7 @@ type DirectOpenParameters = Readonly<{
appBundleId?: string;
execution: ApplicationLifecycleExecution;
terminateRunningApp?: boolean;
background?: boolean;
}>;

function assertOpenPositionals(positionals: readonly string[]): void {
Expand Down Expand Up @@ -219,6 +221,7 @@ async function invokeApplicationUrlOpen(
launchArgs: params.execution.launchArgs ? [...params.execution.launchArgs] : undefined,
terminateRunningApp: params.terminateRunningApp,
url,
...(params.background ? { background: true } : {}),
});
}

Expand All @@ -245,6 +248,7 @@ async function invokeApplicationTargetOpen(
launchConsole: execution.launchConsole,
launchArgs: execution.launchArgs ? [...execution.launchArgs] : undefined,
terminateRunningApp: params.terminateRunningApp,
...(params.background ? { background: true } : {}),
});
}

Expand Down
2 changes: 2 additions & 0 deletions packages/contracts/src/application-lifecycle-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ export type OpenApplicationInput = Readonly<{
runtimeLaunchUrl?: string;
appBundleId?: string;
surface: SessionSurface;
/** Launch or reopen without bringing the app to the front: the session holds this app alone. */
background?: boolean;
hasExistingSession: boolean;
relaunch: boolean;
prewarmRunnerBeforeOpen: boolean;
Expand Down
2 changes: 2 additions & 0 deletions packages/contracts/src/interactor-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,8 @@ export type Interactor = {
launchArgs?: string[];
terminateRunningApp?: boolean;
url?: string;
/** macOS: launch or reopen without bringing the app to the front. */
background?: boolean;
},
): Promise<void>;
openDevice(): Promise<void>;
Expand Down
2 changes: 2 additions & 0 deletions packages/platform-apple/src/core/app-launch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ export async function openIosApp(
terminateRunningApp?: boolean;
url?: string;
runnerOptions?: AppleRunnerCommandOptions;
/** macOS only: launch or reopen without bringing the app to the front. */
background?: boolean;
},
): Promise<void> {
const launchConsole = options?.launchConsole?.trim();
Expand Down
7 changes: 5 additions & 2 deletions packages/platform-apple/src/core/tool-provider-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@ export type ApplePlistProvider = {
readJson(path: string, signal?: AbortSignal): Promise<Record<string, unknown> | null>;
};

export type MacOsOpenOptions = { background?: boolean };

export type AppleMacOsHostProvider = {
openBundle(bundleId: string, url?: string): Promise<void>;
openTarget(target: string): Promise<void>;
/** `background` launches or reopens without bringing the app to the front. */
openBundle(bundleId: string, url?: string, options?: MacOsOpenOptions): Promise<void>;
openTarget(target: string, options?: MacOsOpenOptions): Promise<void>;
readClipboard(): Promise<string>;
writeClipboard(text: string): Promise<void>;
readDarkMode(): Promise<boolean>;
Expand Down
1 change: 1 addition & 0 deletions packages/platform-apple/src/interactor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export function createAppleInteractor(
launchArgs: options?.launchArgs,
terminateRunningApp: options?.terminateRunningApp,
url: options?.url,
background: options?.background,
runnerOptions: runnerOpts,
}),
openDevice: () => openIosDevice(device),
Expand Down
2 changes: 2 additions & 0 deletions packages/platform-apple/src/lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ async function dispatchAppleOpen(
positionals: launch.positionals,
appBundleId: input.appBundleId,
execution: input.execution,
...(input.background ? { background: true } : {}),
...(localIosSimulator && input.relaunch && !input.execution.clearAppState
? { terminateRunningApp: true }
: {}),
Expand Down Expand Up @@ -264,6 +265,7 @@ async function openLaunchUrl(
positionals: [url],
appBundleId: input.appBundleId,
execution,
...(input.background ? { background: true } : {}),
});
}

Expand Down
32 changes: 31 additions & 1 deletion packages/platform-apple/src/os/macos/apps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ vi.mock('node:timers/promises', () => ({ setTimeout: vi.fn(async () => undefined
vi.mock('./helper.ts', () => ({ quitMacOsApp: vi.fn() }));

import { quitMacOsApp } from './helper.ts';
import { closeMacOsApp } from './apps.ts';
import { closeMacOsApp, openMacOsApp } from './apps.ts';
import { createLocalAppleToolProvider, withAppleToolProvider } from '../../core/tool-provider.ts';

const MACOS_DEVICE: DeviceInfo = {
platform: 'apple',
Expand Down Expand Up @@ -76,3 +77,32 @@ test('closeMacOsApp bounds termination confirmation', async () => {
});
expect(mockQuitMacOsApp).toHaveBeenCalledTimes(20);
});

test('openMacOsApp leaves the frontmost app in front only for a background open', async () => {
const opened = async (background: boolean | undefined) => {
const calls: string[][] = [];
const provider = createLocalAppleToolProvider({
runCommand: async (cmd, args) => {
if (cmd === 'open') calls.push(args);
return { exitCode: 0, stdout: '', stderr: '' };
},
});
await withAppleToolProvider(provider, async () => {
await openMacOsApp(MACOS_DEVICE, 'com.example.demo', { background });
await openMacOsApp(MACOS_DEVICE, 'com.example.demo', { url: 'demo://open', background });
await openMacOsApp(MACOS_DEVICE, 'demo://open', { background });
});
return calls;
};

expect(await opened(true)).toEqual([
['-g', '-b', 'com.example.demo'],
['-g', '-b', 'com.example.demo', 'demo://open'],
['-g', 'demo://open'],
]);
expect(await opened(undefined)).toEqual([
['-b', 'com.example.demo'],
['-b', 'com.example.demo', 'demo://open'],
['demo://open'],
]);
});
9 changes: 5 additions & 4 deletions packages/platform-apple/src/os/macos/apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,26 +60,27 @@ export async function resolveMacOsApp(app: string): Promise<string> {
export async function openMacOsApp(
_device: DeviceInfo,
app: string,
options?: { appBundleId?: string; url?: string },
options?: { appBundleId?: string; url?: string; background?: boolean },
): Promise<void> {
const openOptions = { background: options?.background === true };
const explicitUrl = options?.url?.trim();
if (explicitUrl) {
if (!isDeepLinkTarget(explicitUrl)) {
throw new AppError('INVALID_ARGS', 'open <app> <url> requires a valid URL target');
}
const appId = options?.appBundleId ?? (await resolveMacOsApp(app));
await resolveMacOsHostProvider().openBundle(appId, explicitUrl);
await resolveMacOsHostProvider().openBundle(appId, explicitUrl, openOptions);
return;
}

const target = app.trim();
if (isDeepLinkTarget(target)) {
await resolveMacOsHostProvider().openTarget(target);
await resolveMacOsHostProvider().openTarget(target, openOptions);
return;
}

const bundleId = options?.appBundleId ?? (await resolveMacOsApp(target));
await resolveMacOsHostProvider().openBundle(bundleId);
await resolveMacOsHostProvider().openBundle(bundleId, undefined, openOptions);
}

export async function closeMacOsApp(_device: DeviceInfo, app: string): Promise<void> {
Expand Down
26 changes: 26 additions & 0 deletions packages/platform-apple/src/os/macos/helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,32 @@ test('macOS helper screenshot argv carries only --out and --surface', async () =
assert.deepEqual(receivedArgs, ['screenshot', '--out', '/tmp/out.png', '--surface', 'desktop']);
});

test('macOS helper screenshots never run two captures at once', async () => {
let active = 0;
let maxActive = 0;
const provider = createLocalAppleToolProvider({
macosHelper: {
run: async () => {
active += 1;
maxActive = Math.max(maxActive, active);
await new Promise((resolve) => setTimeout(resolve, 10));
active -= 1;
return helperReturn({ path: '/tmp/out.png', surface: 'app' });
},
},
});
const app = macOsHelperSurface('app', 'native')!;

await withAppleToolProvider(provider, async () => {
await Promise.all([
runMacOsScreenshotAction('/tmp/one.png', { surface: app, bundleId: 'com.example.one' }),
runMacOsScreenshotAction('/tmp/two.png', { surface: app, bundleId: 'com.example.two' }),
]);
});

assert.equal(maxActive, 1);
});

test('helper entry points accept only an owner-routed surface', () => {
// Never invoked: each directive fails typecheck once its entry point widens back to an
// unbranded or optional surface.
Expand Down
31 changes: 30 additions & 1 deletion packages/platform-apple/src/os/macos/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
} from '@agent-device/host-kit/host-file';
import {
hostPlatform,
readCurrentOwnerIdentity,
readHostEnvironmentVariable,
writeHostStderr,
} from '@agent-device/host-kit/process';
Expand Down Expand Up @@ -581,5 +582,33 @@ export async function runMacOsScreenshotAction(
}> {
const args = ['screenshot', '--out', outPath];
appendMacOsHelperContextArgs(args, options);
return await runMacOsHelper(args, { signal: options.signal });
return await withMacOsScreenCaptureLock(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The screenshot's AbortSignal is only forwarded into the inner helper run, not into the lock acquisition. withMacOsScreenCaptureLock waits up to MACOS_HELPER_TIMEOUT_MS + 5_000 (35s) for contention via host-kit's acquireProcessLock, which takes no signal and polls by sleeping, so a cancelled screenshot stays blocked for the whole wait instead of aborting promptly (previously the signal reached the helper directly). It then either runs the helper against an already-aborted signal or fails with a lock timeout. Consider letting the caller abort the lock wait, e.g. accepting an AbortSignal in withMacOsScreenCaptureLock and racing the acquisition poll against it (which needs a host-kit signal-aware acquire).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/platform-apple/src/os/macos/helper.ts, line 585:

<comment>The screenshot's AbortSignal is only forwarded into the inner helper run, not into the lock acquisition. `withMacOsScreenCaptureLock` waits up to `MACOS_HELPER_TIMEOUT_MS + 5_000` (35s) for contention via host-kit's `acquireProcessLock`, which takes no signal and polls by sleeping, so a cancelled screenshot stays blocked for the whole wait instead of aborting promptly (previously the signal reached the helper directly). It then either runs the helper against an already-aborted signal or fails with a lock timeout. Consider letting the caller abort the lock wait, e.g. accepting an `AbortSignal` in `withMacOsScreenCaptureLock` and racing the acquisition poll against it (which needs a host-kit signal-aware acquire).</comment>

<file context>
@@ -581,5 +582,33 @@ export async function runMacOsScreenshotAction(
   const args = ['screenshot', '--out', outPath];
   appendMacOsHelperContextArgs(args, options);
-  return await runMacOsHelper(args, { signal: options.signal });
+  return await withMacOsScreenCaptureLock(
+    async () => await runMacOsHelper(args, { signal: options.signal }),
+  );
</file context>

async () => await runMacOsHelper(args, { signal: options.signal }),
);
}

/**
* ScreenCaptureKit fails or hangs window captures that two processes start at the same time
* (measured on macOS 27 with two helpers capturing two apps), so every helper capture on this Mac
* runs alone, whichever daemon asks. The lock sits in the host-global device claims directory,
* the store every daemon on the host already coordinates through.
*/
async function withMacOsScreenCaptureLock<T>(task: () => Promise<T>): Promise<T> {
const lockRoot =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This re-implements resolveDeviceClaimRoot() from src/daemon/device/device-claim-paths.ts. The two copies can drift, and the lock only lands in the claims directory by convention. Reuse one resolver, or move it to a shared package both sides can import.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/platform-apple/src/os/macos/helper.ts, line 597:

<comment>This re-implements `resolveDeviceClaimRoot()` from `src/daemon/device/device-claim-paths.ts`. The two copies can drift, and the lock only lands in the claims directory by convention. Reuse one resolver, or move it to a shared package both sides can import.</comment>

<file context>
@@ -581,5 +582,33 @@ export async function runMacOsScreenshotAction(
+ * the store every daemon on the host already coordinates through.
+ */
+async function withMacOsScreenCaptureLock<T>(task: () => Promise<T>): Promise<T> {
+  const lockRoot =
+    readHostEnvironmentVariable('AGENT_DEVICE_CLAIMS_DIR')?.trim() ||
+    path.join(hostHomeDirectory(), '.agent-device', 'device-claims');
</file context>

readHostEnvironmentVariable('AGENT_DEVICE_CLAIMS_DIR')?.trim() ||
path.join(hostHomeDirectory(), '.agent-device', 'device-claims');
await ensureHostDirectory(lockRoot);
const { acquireProcessLock, withProcessLock } = await import('@agent-device/host-kit/file');
return await withProcessLock({
acquire: async () => {
const owner = readCurrentOwnerIdentity();
return await acquireProcessLock({
lockDirPath: path.join(path.resolve(lockRoot), 'macos-screen-capture.lock'),
owner: { pid: owner.pid, startTime: owner.startTime, acquiredAtMs: Date.now() },
timeoutMs: MACOS_HELPER_TIMEOUT_MS + 5_000,
description: 'the macOS screen capture lock',
});
},
task,
});
}
Loading
Loading