Add guided simbroker setup - #19
Conversation
Why: Make first-time machine setup safe, obvious, idempotent, and shared by the CLI and macOS app. Changed: Add deterministic setup preview/apply planning, prerequisite checks, six-device provisioning, service and health completion, app confirmation UI, portable project defaults, documentation, smoke coverage, and active specifications. Verification: Passed implementation and spec-only harness profiles, broker-core, client, docs, harness-adoption, app, install smoke, package smoke, public-surface, and macOS build verification. Affected: broker-core, client, app, docs, specs, examples, packaging, Homebrew metadata, and install smoke. Refs: GSB-SETUP-001 Session: task-sessions/guided-setup
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: deb89d5e5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Why: - Address the safety, readiness, validation, and recovery gaps identified during PR #19 review. Changed: - Make provisioning cooperatively cancellable and attribute rollback only to devices created by the active setup attempt. - Validate existing-host snapshots, runtime inputs, recovery guidance, and major-only host requirements. - Preserve actionable setup failures in the macOS confirmation sheet and clarify next-Alpha artifact availability. - Add focused broker, CLI, signal, formatter, and Swift regression coverage. Verification: - npm run test:broker-core - npm run test:client - npm run test:docs - npm run test:harness-adoption - npm run test:app - npm run test:install-smoke - npm run test:package-smoke - npm run verify:public-surface - ./script/build_and_run.sh --verify - agent verify profiles: implementation, spec-only Affected: - README.md - app/ - broker-core/ - client/ Refs: - #19 Session: - task-sessions/guided-setup-review
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3b17a0b01
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ```bash | ||
| npm install -g https://github.com/fiveonecode/simulator-broker/releases/download/v0.1.0-alpha.2/simbroker-0.1.0-alpha.2.tgz | ||
| simbroker --help | ||
| simbroker setup |
There was a problem hiding this comment.
When users follow this package-specific installation block, the immutable 0.1.0-alpha.2 archive is installed and the next command fails because that release predates simbroker setup. Fresh evidence after the earlier documentation fix is that this changed README still directly pairs the old release URL with the new command, without the availability warning added to the root README; point this example at an artifact containing setup or clearly scope it to a source build.
Useful? React with 👍 / 👎.
| prerequisites: [...corePreview.prerequisites, { | ||
| details: error?.payload ?? { error: error?.message ?? String(error) }, | ||
| id: "service-identity", | ||
| remediationCommands: ["simbroker service stop", "simbroker setup"], |
There was a problem hiding this comment.
When the selected socket reaches a broker using different host or state paths, the advertised simbroker service stop recovery command uses the same selected/default paths and stopService immediately calls the identity-asserting serviceStatus, so it fails with the same service-identity-mismatch before sending a stop request. Fresh evidence after the earlier nextSteps fix is this unchanged stop path at client/bin/simbroker.mjs:424-425; recovery must use the reported service identity or otherwise provide a stop mechanism that can handle this mismatch.
Useful? React with 👍 / 👎.
| case .ready: | ||
| return [ | ||
| formatter.command("host status --host-config \"\(store.hostConfigPath)\" --state-root \"\(store.stateRootPath)\""), | ||
| formatter.command("setup --host-config \"\(store.hostConfigPath)\" --state-root \"\(store.stateRootPath)\""), |
There was a problem hiding this comment.
When the app was launched with a --service-socket override, this displayed fallback drops that override even though guidedSetupPathArguments() passes it to the in-app setup invocation. Copying the command therefore probes and starts brokerd on the socket derived from stateRootPath rather than the socket the app uses; with an existing broker on the configured socket this can start a second service against the same host/state or leave the app unable to connect to the newly started service. Append the configured --service-socket value to the fallback command.
Useful? React with 👍 / 👎.
| } | ||
| process.stdout.write(payload.planId); | ||
| ' "$(artifact_path setup-preview.json)")" | ||
| log_command setup-apply simbroker setup --apply --confirm "$setup_plan_id" --host-id install-smoke-host --json |
There was a problem hiding this comment.
When confirmed setup starts brokerd but then fails during snapshot refresh or health verification, set -e exits from this log_command before service_started=1 executes. The EXIT cleanup consequently skips service stop, deletes the temporary state tree, and leaves the detached brokerd process running against removed paths after the smoke test fails. Mark cleanup as required before invoking setup, or have cleanup probe the temporary socket regardless of the flag.
Useful? React with 👍 / 👎.
1 participant
Summary
simbroker setupflow for machine prerequisites, runtime selection, six-device planning, confirmation, provisioning, service startup, snapshot refresh, and health verification--ios-versionis explicitly suppliedSafety and recovery
Verification
npm run test:broker-core— 254 passednpm run test:client— 186 passednpm run test:docs— 19 passednpm run test:harness-adoption— 24 passednpm run test:app— 84 passednpm run test:install-smokenpm run test:package-smokenpm run verify:public-surface./script/build_and_run.sh --verifyimplementationandspec-onlyagent verification profilesSpecification
The implemented contract is documented in
spec/tasks/guided-simbroker-setup.md(GSB-SETUP-001).