Skip to content

Commit 474bd0f

Browse files
committed
fix: address Android W5 review feedback
1 parent 424610d commit 474bd0f

7 files changed

Lines changed: 59 additions & 19 deletions

File tree

docs/adr/0019-request-bound-platform-runtime.md

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,9 @@ execution, diagnostics, retry, probes, locks, foreground Apple tooling, physical
183183
constructed by exactly one composition root. No current issue owns migrating the runner's direct
184184
consumers behind the composition gateway; if such a migration retires them, the `./runner` seam
185185
narrows with it, but the facet itself is the intended ownership model, not a temporary exception.
186-
The declaration mechanism stays apple-specific until another family needs a mechanics facet.
186+
Mechanics-facet declarations are explicit per family: the Apple runner and Android mechanics/host
187+
facets are enumerated above, and a new family adds its own named facet only with an owning consumer
188+
and evidence.
187189

188190
Canonical family, `AppleOS`, public-leaf, and selector identity remain declared in
189191
`@agent-device/kernel/device`. Platform-module metadata references one canonical family; during
@@ -678,6 +680,31 @@ removed against package bytes added; net growth is exceptional and each contribu
678680
named and justified individually. Contract modules stay vocabulary-thin under the existing
679681
capture-kit rule; per-unit type inflation is size growth and is reviewed as such.
680682

683+
#### W5 Android family accounting (PR #2117)
684+
685+
The W5 exact-head size report at `9fa2778` against `437465f37` reported these rounded CI values:
686+
687+
| Metric | Original baseline | W5 head | W5 delta |
688+
| ------------------ | ----------------: | ------: | -------: |
689+
| Raw JavaScript | 2.48 MB | 2.49 MB | +8.2 kB |
690+
| Gzipped JavaScript | 835.0 kB | 831.2 kB | -3.9 kB |
691+
| npm tarball | 958.5 kB | 959.6 kB | +1.1 kB |
692+
| npm unpacked | 3.32 MB | 3.33 MB | +9.0 kB |
693+
694+
At source level, the moved Android production tree accounts for 456,492 B removed from
695+
`src/platforms/android` versus 475,413 B in changed package targets, a +18,921 B relocation
696+
delta; the new root host/facade seam adds 19,978 B. The published +9.0 kB unpacked result is the
697+
package result, not a source-byte estimate. The increase is attributable to the named `mechanics`
698+
closure, the injected root host composition, and preserving deferred helper loading while root
699+
chunks are removed or repartitioned. The largest packed increase is the mechanics chunk; it is
700+
offset by deletion or shrinkage of the superseded root Android chunks.
701+
702+
A smaller design that retained root re-export shims, a broad barrel, or ambient filesystem/process
703+
access would reduce gross source movement but violate this ADR's package-closure,
704+
no-compatibility-shim, named-facet, host-injection, and implementation-lazy constraints. This is
705+
exceptional W5 accounting, not reusable checkpoint headroom; after rebasing on #2116, the
706+
combined-family CI size result remains the authoritative current measurement.
707+
681708
Raw per-unit parity, planted-red, and size evidence belongs in #1739 and its PRs; this ADR retains
682709
the decision, the evidence tiers, and the rule that each unit must justify exceptional growth
683710
without treating checkpoint budget as an allowance.

packages/platform-android/src/adb-host.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,6 @@ import type {
1212
AndroidAdbSpawnOptions,
1313
} from './adb-transport.ts';
1414

15-
// R13 bars platform packages from raw process, fs, and ambient host authority; the adb/IME
16-
// cluster reaches those primitives only through this explicitly injected host port. The root
17-
// composition wiring (src/platform-runtime-android-adb-host.ts) binds it before any consumer
18-
// can call into the cluster.
19-
2015
export type AndroidAdbCommandExecutorOverride = (
2116
cmd: string,
2217
args: string[],

scripts/check-affected/model.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,25 @@ test('Android helper change selects the android-helpers build', () => {
123123
]);
124124
});
125125

126+
test('Android package test fixture selects the unit suite instead of failing open', () => {
127+
const fixture =
128+
'packages/platform-android/src/__tests__/test-utils/fixtures/android-helper-apk.fixture';
129+
const result = plan([fixture]);
130+
assert.equal(result.failOpen, false);
131+
assert.deepEqual(result.checks, ['unit']);
132+
assert.deepEqual(
133+
result.reasons.filter((reason) => reason.rule === 'own:android-package-test-fixture'),
134+
[
135+
{
136+
check: 'unit',
137+
path: fixture,
138+
rule: 'own:android-package-test-fixture',
139+
detail: 'the Android package test fixture is consumed by the unit suite',
140+
},
141+
],
142+
);
143+
});
144+
126145
test('MCP metadata change selects the mcp-metadata check', () => {
127146
assert.deepEqual(ids(['server.json']), ['mcp-metadata']);
128147
});

scripts/check-affected/model.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,14 @@ const BUILD_OWNERSHIP: ReadonlyArray<{
481481
owns: (file) =>
482482
file.startsWith('android/snapshot-helper/') || file.startsWith('android/ime-helper/'),
483483
},
484+
{
485+
check: 'unit',
486+
rule: 'own:android-package-test-fixture',
487+
detail: 'the Android package test fixture is consumed by the unit suite',
488+
owns: (file) =>
489+
file ===
490+
'packages/platform-android/src/__tests__/test-utils/fixtures/android-helper-apk.fixture',
491+
},
484492
{
485493
check: 'macos-helper',
486494
rule: 'own:macos-helper',

scripts/layering/platform-package-policy.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,9 @@ export function checkPlatformsRootShape(files: readonly string[]): LayeringViola
6060
rule: 'platforms-root-shape',
6161
file,
6262
line: 1,
63-
message:
64-
file.startsWith('src/platforms/android/')
65-
? 'the Android family has moved to packages/platform-android; remove the superseded src/platforms/android path'
66-
: 'src/platforms may hold only the remaining apple family directory and __tests__; retired family code belongs in its platform package and shared code belongs in a substrate package',
63+
message: file.startsWith('src/platforms/android/')
64+
? 'the Android family has moved to packages/platform-android; remove the superseded src/platforms/android path'
65+
: 'src/platforms may hold only the remaining apple family directory and __tests__; retired family code belongs in its platform package and shared code belongs in a substrate package',
6766
}));
6867
}
6968
const APPLE_RUNNER_FACADE = '@agent-device/platform-apple/runner';

src/daemon/__tests__/request-platform-providers.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
runXcrun,
1919
} from '../../platforms/apple/core/tool-provider.ts';
2020
import type { AndroidAdbExecutor } from '@agent-device/platform-android/mechanics';
21-
import { resolveWebProvider, type WebProvider } from '../../platforms/web/provider.ts';
21+
import { resolveWebProvider, type WebProvider } from '@agent-device/platform-web';
2222
import {
2323
resolveAppleRunnerScreenRecordingTransport,
2424
type AppleRunnerScreenRecordingTransport,

src/platform-runtime-android-adb-host.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ import {
2525
import os from 'node:os';
2626
import path from 'node:path';
2727

28-
// Composition wiring for the extracted Android family: the package owns transport and IME
29-
// semantics; this file injects the raw host primitives R13 bars from platform packages.
3028
bindAndroidAdbHost({
3129
environment: process.env,
3230
files: {
@@ -70,7 +68,6 @@ bindAndroidAdbHost({
7068
directoryHandle = await open(directory, 'r');
7169
await directoryHandle.sync();
7270
} catch {
73-
// Atomic rename remains the correctness boundary where directory sync is unavailable.
7471
} finally {
7572
if (directoryHandle) await directoryHandle.close().catch(() => {});
7673
}
@@ -82,15 +79,10 @@ bindAndroidAdbHost({
8279
writeBytes: async (filePath, value) => await writeFile(filePath, value),
8380
},
8481
execSerialAdb: async (serial, args, options) =>
85-
// Local adb execution must escape any active provider scope to avoid routing
86-
// tunnel-backed providers back into themselves when they shell out to adb.
8782
await withoutCommandExecutorOverride(
8883
async () =>
8984
await runCmd('adb', ['-s', serial, ...args], {
9085
...options,
91-
// Some `adb shell` children can survive killing the adb parent and keep
92-
// requests open past timeout. Give each adb call its own process group
93-
// so timeout/abort cleanup can tear down the whole local command tree.
9486
detached: process.platform !== 'win32',
9587
}),
9688
),

0 commit comments

Comments
 (0)