Skip to content

Commit 5969bc8

Browse files
autopilot: address PR #19 review feedback
Why: - Resolve actionable GitHub review feedback for PR #19. Changed: - Setup now fail-closes on snapshot-incomplete live leases and pins, and on schema-invalid persisted registry alias data, instead of finishing those hosts as ready. Verification: - npm run agent:complete -- --session-dir [controller artifact: jobs/pr-19/20260825-171439-3657ed81-cb02-408b-9e4f-0021828b8f1e/task-session] - npm run agent:context -- --paths-file [controller artifact: jobs/pr-19/20260825-171439-3657ed81-cb02-408b-9e4f-0021828b8f1e/actual-paths.txt] --session-dir [controller artifact: jobs/pr-19/20260825-171439-3657ed81-cb02-408b-9e4f-0021828b8f1e/task-session] - npm run agent:verify -- --profile implementation --paths-file [controller artifact: jobs/pr-19/20260825-171439-3657ed81-cb02-408b-9e4f-0021828b8f1e/task-session/verify/implementation/paths.txt] --session-dir [controller artifact: jobs/pr-19/20260825-171439-3657ed81-cb02-408b-9e4f-0021828b8f1e/task-session] - npm run agent:verify -- --profile spec-only --paths-file [controller artifact: jobs/pr-19/20260825-171439-3657ed81-cb02-408b-9e4f-0021828b8f1e/task-session/verify/spec-only/paths.txt] --session-dir [controller artifact: jobs/pr-19/20260825-171439-3657ed81-cb02-408b-9e4f-0021828b8f1e/task-session] Affected: - .github/workflows/ci.yml - CHANGELOG.md - CONTRIBUTING.md - Casks/simulator-broker.rb - Formula/simbroker.rb - README.md - agent-harness/package-lock.json - agent-harness/package.json - app/README.md - app/Sources/BrokerDashboardStore.swift - app/Sources/BrokerLocalCommandClient.swift - app/Sources/BrokerOnboardingCommands.swift - app/Sources/BrokerServiceClient.swift - app/Sources/BrokerSetupModels.swift - app/Sources/BrokerSnapshotLoader.swift - app/Sources/RootView.swift - app/Sources/SetupPlanDevicesView.swift - app/Sources/SetupPlanPrerequisitesView.swift - app/Sources/SetupPlanSheet.swift - app/Sources/SharedViews.swift - app/Tests/BrokerDashboardStoreTests.swift - app/Tests/BrokerLocalCommandClientTests.swift - app/Tests/BrokerOnboardingCommandsTests.swift - app/Tests/BrokerRuntimePathsTests.swift - app/Tests/BrokerServiceClientTests.swift - app/Tests/BrokerSnapshotLoaderTests.swift - broker-core/containment.mjs - broker-core/error-contract.mjs - broker-core/index.mjs - broker-core/simctl.mjs - broker-core/test/broker-core.test.mjs - client/bin/simbroker.mjs - client/command-dispatch.mjs - client/public-surface.mjs - client/service/service-client.mjs - client/setup-preflight.mjs - client/setup-provisioning-worker.mjs - client/setup-provisioning.mjs - client/test/brokerd.test.mjs - client/test/install-distribution.test.mjs - client/test/public-surface.test.mjs - client/test/setup-preflight.test.mjs - client/test/simbroker.test.mjs - docs/concepts.md - docs/getting-started.md - docs/status.md - docs/test/front-door.test.mjs - examples/harness-adoption/README.md - examples/harness-adoption/sample-consumer-repo/.simulator-broker/project.json - examples/harness-adoption/sample-consumer-repo/README.md - packages/simbroker/README.md - scripts/install_smoke.sh - scripts/package_distribution.sh - spec/README.md - spec/architecture.md - spec/build-and-test.md - spec/global-simulator-broker.md - spec/project-structure.md - spec/tasks/README.md - spec/tasks/guided-simbroker-setup.md Refs: - #19 - #19 (comment) - #19 (comment) - #19 (comment) Session: - task-session: [controller artifact: jobs/pr-19/20260825-171439-3657ed81-cb02-408b-9e4f-0021828b8f1e/task-session] - report: [controller artifact: jobs/pr-19/20260825-171439-3657ed81-cb02-408b-9e4f-0021828b8f1e/report.md]
1 parent b601997 commit 5969bc8

4 files changed

Lines changed: 173 additions & 15 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2525

2626
- Guided setup binds existing-host confirmation to the requested `--host-id`,
2727
uses the separate device-type inventory when a runtime omits
28-
`supportedDeviceTypes`, and fails closed when known-projects or lease/pin
29-
JSON cannot be loaded or is not a complete identity record instead of
30-
reporting `ready`. A missing host-config with an existing registry, lease,
31-
or pin record in the selected state root is blocked rather than treated as
32-
a confirmable fresh plan. Preview probes service identity even when no host
33-
is configured yet, so a socket occupied by a different broker is blocked
34-
before device creation.
28+
`supportedDeviceTypes`, and fails closed when known-projects, registry, or
29+
lease/pin JSON cannot be loaded or is not a complete identity record instead
30+
of reporting `ready`. Incomplete live leases that omit snapshot-required
31+
fields, pins that omit project identity, and persisted registry alias data
32+
with an unrecognized `health` value or no configured host aliases stay
33+
blocked instead of being normalized during automatic finishing. A missing
34+
host-config with an existing registry, lease, or pin record in the selected
35+
state root is blocked rather than treated as a confirmable fresh plan.
36+
Preview probes service identity even when no host is configured yet, so a
37+
socket occupied by a different broker is blocked before device creation.
3538
`ready` also requires the app snapshot to be at least as new as
3639
known-projects, lease/pin records, and the lease/pin directories, and to
3740
match the current doctor-record set, so deletions are finishing work rather

broker-core/index.mjs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1688,6 +1688,7 @@ function setupExistingHostState(paths, inventory, options = {}) {
16881688
registryMissing = true;
16891689
registry = normalizeRegistry(null, hostConfig, nowIso(options.now));
16901690
} else {
1691+
assertValidPersistedRegistry(persistedRegistry, hostConfig);
16911692
registry = normalizeRegistry(persistedRegistry, hostConfig, nowIso(options.now));
16921693
}
16931694
} catch (error) {
@@ -2154,6 +2155,37 @@ function normalizeKnownProjects(rawKnownProjects, timestamp) {
21542155
};
21552156
}
21562157

2158+
function assertValidPersistedRegistry(rawRegistry, hostConfig) {
2159+
requireObject(rawRegistry, "registry");
2160+
const rawAliases = requireObject(rawRegistry.aliases, "registry.aliases");
2161+
if (
2162+
hostConfig.aliases.length > 0
2163+
&& !hostConfig.aliases.some((hostAlias) => Object.hasOwn(rawAliases, hostAlias.alias))
2164+
) {
2165+
throw new BrokerError("registry.aliases must include at least one configured host alias.", {
2166+
field: "registry.aliases",
2167+
reasonCode: "invalid-config",
2168+
});
2169+
}
2170+
for (const hostAlias of hostConfig.aliases) {
2171+
if (!Object.hasOwn(rawAliases, hostAlias.alias)) {
2172+
continue;
2173+
}
2174+
const fieldPrefix = `registry.aliases.${hostAlias.alias}`;
2175+
const entry = requireObject(rawAliases[hostAlias.alias], fieldPrefix);
2176+
if (Object.hasOwn(entry, "health") && !ALLOWED_HEALTH_STATES.has(entry.health)) {
2177+
throw new BrokerError(
2178+
`${fieldPrefix}.health must be one of ${[...ALLOWED_HEALTH_STATES].join(", ")}.`,
2179+
{
2180+
field: `${fieldPrefix}.health`,
2181+
reasonCode: "invalid-config",
2182+
},
2183+
);
2184+
}
2185+
}
2186+
return rawRegistry;
2187+
}
2188+
21572189
function normalizeRegistry(rawRegistry, hostConfig, timestamp) {
21582190
if (rawRegistry === null) {
21592191
rawRegistry = {
@@ -2411,7 +2443,11 @@ function assertValidLeaseRecord(record) {
24112443
requireString(record.actorType, "lease.actorType");
24122444
requireString(record.actorId, "lease.actorId");
24132445
requireString(record.projectId, "lease.projectId");
2446+
requireString(record.projectName, "lease.projectName");
24142447
requireString(record.purposeId, "lease.purposeId");
2448+
requireString(record.displayName, "lease.displayName");
2449+
requireString(record.leaseKind, "lease.leaseKind");
2450+
requireString(record.repoRoot, "lease.repoRoot");
24152451
requireString(record.startedAt, "lease.startedAt");
24162452
requirePositiveIntegerField(record.ownerPid, "lease.ownerPid");
24172453
return record;
@@ -2421,6 +2457,12 @@ function assertValidPinRecord(record) {
24212457
requireObject(record, "pin");
24222458
requireString(record.pinId, "pin.pinId");
24232459
requireString(record.alias, "pin.alias");
2460+
requireString(record.projectId, "pin.projectId");
2461+
requireString(record.projectName, "pin.projectName");
2462+
requireString(record.actorId, "pin.actorId");
2463+
requireString(record.actorType, "pin.actorType");
2464+
requireString(record.createdAt, "pin.createdAt");
2465+
requireString(record.repoRoot, "pin.repoRoot");
24242466
return record;
24252467
}
24262468

broker-core/test/broker-core.test.mjs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,6 +1234,111 @@ test("setup blocks an existing host when doctor-relevant state files are malform
12341234
assert.equal(fs.existsSync(path.join(resolvedPaths.leasesDir, "partial-lease.json")), true);
12351235
});
12361236

1237+
test("setup blocks snapshot-incomplete leases, pins, and schema-invalid registry alias data", () => {
1238+
const paths = makePaths();
1239+
const resolvedPaths = brokerPaths(paths);
1240+
const preview = previewSetupBroker(resolvedPaths, {
1241+
hostId: "snapshot-schema-setup",
1242+
simctlAdapter: paths.simctl.adapter,
1243+
});
1244+
applySetupBroker(resolvedPaths, {
1245+
confirmPlanId: preview.planId,
1246+
hostId: "snapshot-schema-setup",
1247+
simctlAdapter: paths.simctl.adapter,
1248+
});
1249+
1250+
const incompleteLeasePath = path.join(resolvedPaths.leasesDir, "incomplete-snapshot-lease.json");
1251+
writeJson(incompleteLeasePath, {
1252+
actorId: "agent:1",
1253+
actorType: "agent",
1254+
alias: "ui-1",
1255+
leaseId: "incomplete-snapshot-lease",
1256+
ownerPid: process.pid,
1257+
projectId: "demo-app",
1258+
purposeId: "agent-ui-session",
1259+
simulatorId: "sim-ui-1",
1260+
startedAt: "2026-01-01T00:00:00.000Z",
1261+
});
1262+
const incompleteLeaseBlocked = previewSetupBroker(resolvedPaths, {
1263+
simctlAdapter: paths.simctl.adapter,
1264+
});
1265+
assert.equal(incompleteLeaseBlocked.status, "blocked");
1266+
assert.ok(incompleteLeaseBlocked.prerequisites.some((issue) =>
1267+
issue.id === "leases" && issue.status === "blocked"));
1268+
assert.throws(() => applySetupBroker(resolvedPaths, {
1269+
confirmPlanId: incompleteLeaseBlocked.planId,
1270+
simctlAdapter: paths.simctl.adapter,
1271+
}), (error) => error.payload?.reasonCode === "setup-prerequisite-failed");
1272+
assert.equal(fs.existsSync(incompleteLeasePath), true);
1273+
fs.rmSync(incompleteLeasePath);
1274+
1275+
const incompletePinPath = path.join(resolvedPaths.pinsDir, "incomplete-pin.json");
1276+
writeJson(incompletePinPath, {
1277+
alias: "ui-1",
1278+
pinId: "incomplete-pin",
1279+
});
1280+
const incompletePinBlocked = previewSetupBroker(resolvedPaths, {
1281+
simctlAdapter: paths.simctl.adapter,
1282+
});
1283+
assert.equal(incompletePinBlocked.status, "blocked");
1284+
assert.ok(incompletePinBlocked.prerequisites.some((issue) =>
1285+
issue.id === "leases" && issue.status === "blocked"));
1286+
assert.throws(() => applySetupBroker(resolvedPaths, {
1287+
confirmPlanId: incompletePinBlocked.planId,
1288+
simctlAdapter: paths.simctl.adapter,
1289+
}), (error) => error.payload?.reasonCode === "setup-prerequisite-failed");
1290+
assert.deepEqual(readJson(incompletePinPath), {
1291+
alias: "ui-1",
1292+
pinId: "incomplete-pin",
1293+
});
1294+
fs.rmSync(incompletePinPath);
1295+
1296+
const originalRegistry = readJson(resolvedPaths.registryPath);
1297+
const invalidHealthRegistry = structuredClone(originalRegistry);
1298+
invalidHealthRegistry.aliases["ui-1"].health = "not-a-health";
1299+
writeJson(resolvedPaths.registryPath, invalidHealthRegistry);
1300+
const invalidHealthBlocked = previewSetupBroker(resolvedPaths, {
1301+
simctlAdapter: paths.simctl.adapter,
1302+
});
1303+
assert.equal(invalidHealthBlocked.status, "blocked");
1304+
assert.ok(invalidHealthBlocked.prerequisites.some((issue) =>
1305+
issue.id === "registry" && issue.status === "blocked"));
1306+
assert.throws(() => applySetupBroker(resolvedPaths, {
1307+
confirmPlanId: invalidHealthBlocked.planId,
1308+
simctlAdapter: paths.simctl.adapter,
1309+
}), (error) => error.payload?.reasonCode === "setup-prerequisite-failed");
1310+
assert.equal(readJson(resolvedPaths.registryPath).aliases["ui-1"].health, "not-a-health");
1311+
1312+
writeJson(resolvedPaths.registryPath, {
1313+
aliases: {},
1314+
updatedAt: originalRegistry.updatedAt,
1315+
version: originalRegistry.version,
1316+
});
1317+
const emptyAliasMapBlocked = previewSetupBroker(resolvedPaths, {
1318+
simctlAdapter: paths.simctl.adapter,
1319+
});
1320+
assert.equal(emptyAliasMapBlocked.status, "blocked");
1321+
assert.ok(emptyAliasMapBlocked.prerequisites.some((issue) =>
1322+
issue.id === "registry" && issue.status === "blocked"));
1323+
assert.throws(() => applySetupBroker(resolvedPaths, {
1324+
confirmPlanId: emptyAliasMapBlocked.planId,
1325+
simctlAdapter: paths.simctl.adapter,
1326+
}), (error) => error.payload?.reasonCode === "setup-prerequisite-failed");
1327+
assert.deepEqual(readJson(resolvedPaths.registryPath).aliases, {});
1328+
1329+
const missingHealthRegistry = structuredClone(originalRegistry);
1330+
delete missingHealthRegistry.aliases["ui-1"].health;
1331+
writeJson(resolvedPaths.registryPath, missingHealthRegistry);
1332+
const missingHealthAllowed = previewSetupBroker(resolvedPaths, {
1333+
simctlAdapter: paths.simctl.adapter,
1334+
});
1335+
assert.notEqual(missingHealthAllowed.status, "blocked");
1336+
assert.equal(
1337+
missingHealthAllowed.prerequisites.some((issue) => issue.id === "registry" && issue.status === "blocked"),
1338+
false,
1339+
);
1340+
});
1341+
12371342
test("setup blocks a missing host config when the state root already has broker records", () => {
12381343
const paths = makePaths();
12391344
const resolvedPaths = brokerPaths(paths);

spec/tasks/guided-simbroker-setup.md

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Guided `simbroker setup`
22

33
> **Document ID:** `GSB-SETUP-001`
4-
> **Version:** `1.0.8`
4+
> **Version:** `1.0.9`
55
> **Last Updated:** `2026-08-25`
66
> **Status:** `Active`
77
> **Owner:** `spec-steward`, `ios-dev`
@@ -184,13 +184,20 @@ Apply performs, in order:
184184
- Health failure preserves host/service and reports doctor issues plus exact
185185
per-alias repair commands.
186186
- Existing invalid/unhealthy host is blocked without repair/replacement,
187-
lease/pin mutation, or retirement. Invalid known-projects or lease/pin JSON
188-
is the same class of existing-host failure: diagnose with `simbroker doctor`,
189-
do not report `ready`. Valid JSON that is not a complete lease or pin
190-
identity record, such as `{}` or a lease with only `leaseId` and `alias`,
191-
is invalid existing-host state. Setup does not treat a missing host-config
192-
as a confirmable fresh plan when the selected state root already contains a
193-
registry file or lease/pin JSON.
187+
lease/pin mutation, or retirement. Invalid known-projects, registry, or
188+
lease/pin JSON is the same class of existing-host failure: diagnose with
189+
`simbroker doctor`, do not report `ready`. Valid JSON that is not a complete
190+
lease or pin identity record, such as `{}`, a lease with only `leaseId` and
191+
`alias`, a live lease that omits snapshot-required fields such as
192+
`displayName`, `leaseKind`, `projectName`, or `repoRoot`, or a pin that has
193+
`pinId` and `alias` but omits `projectId`, `projectName`, `actorId`,
194+
`actorType`, `createdAt`, or `repoRoot`, is invalid existing-host state.
195+
Persisted registry alias data that is schema-invalid, such as an
196+
unrecognized `health` value or an alias map that contains none of the
197+
configured host aliases, is also invalid; setup must not normalize it into a
198+
`ready` host. Setup does not treat a missing host-config as a confirmable
199+
fresh plan when the selected state root already contains a registry file or
200+
lease/pin JSON.
194201
- Preview probes the requested service socket even when no host is configured
195202
yet. A running broker with a different host-config, state-root, or socket
196203
identity is a `service-identity` blocker and is not confirmable, so apply
@@ -418,6 +425,7 @@ long-running plan/handoff/evaluation, and a passing `agent:complete`.
418425

419426
| Version | Date | Author | Changes |
420427
|---|---|---|---|
428+
| 1.0.9 | 2026-08-25 | `spec-steward`, `ios-dev` | Fail closed on snapshot-incomplete lease/pin records and schema-invalid persisted registry alias data instead of normalizing them to `ready` |
421429
| 1.0.8 | 2026-08-25 | `spec-steward`, `ios-dev` | Block partial lease records and missing-host state roots that already contain registry/lease/pin artifacts, keep selected paths on service-identity retries, and refresh the app dashboard on an already-ready setup preview |
422430
| 1.0.7 | 2026-08-25 | `spec-steward`, `ios-dev` | App Stop/timeout SIGTERM window includes rollback attribution inventory and the extra inventory after an indeterminate create failure |
423431
| 1.0.6 | 2026-08-25 | `spec-steward`, `ios-dev` | Fail closed on schema-invalid lease/pin JSON, keep fallback device types order-stable, exclude automatic finishing from the setup sheet, treat doctor-record deletions as snapshot finishing work, and preserve selected path overrides in recovery commands |

0 commit comments

Comments
 (0)