Require explicit Apps Platform identity in bb - #297
Conversation
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES: the new create sequence can durably reserve an app and then fail without a supported way to resume, leaving the requested identity stranded. The supplied GitHub evidence is structurally available and all listed check runs completed successfully; required checks still independently govern merge readiness.
Deterministic publication result: 1 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES: recovery can initialize an intentionally reserve-only non-static app against the current plan, and its reservation predicate treats missing or malformed version evidence as proof that no versions exist. The prior partial-success concern was reassessed against the new implementation; one remaining instance is suppressed to its existing unresolved thread. Supplied GitHub checks are valid but one was still in progress, so required checks continue to govern merge readiness independently.
Deterministic publication result: 2 blocking and 0 non-blocking inline finding(s) publishable; 1 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES: the create flow accepts a partial or malformed initialization decision and can durably reserve an app while silently treating the missing decision as false. The prior recovery-provenance concern remains protected by a resolved thread with substantive human replies and is suppressed. Supplied GitHub evidence is structurally valid and all listed checks completed successfully; required checks still independently govern merge readiness.
Deterministic publication result: 1 blocking and 0 non-blocking inline finding(s) publishable; 1 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
APPROVE: the exact-ID create, reserve, initialization, and recovery flows are coherent at this SHA, and the prior blocking findings are addressed. One remaining recovery-provenance concern is protected by a resolved thread with substantive human replies and is suppressed. Supplied GitHub evidence is structurally valid and all listed checks completed successfully; the companion control-plane change must still be deployed first as documented, and required checks independently govern merge readiness.
Deterministic publication result: 0 blocking and 0 non-blocking inline finding(s) publishable; 1 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
Pending checks: 1 check(s) are not complete.
This approval reflects the completed code review only; merge readiness remains governed by the repository's required checks.
Summary
Apps Platform owns the final app identity and the decision to initialize it.
bb apps createneeds to preserve both decisions without accepting a substitute ID, mutating from a partial plan, or leaving an interrupted create permanently stranded.Create now requires a non-empty
--app-idor--name, plans the identity, and reserves the exact planned ID before optional initialization. It rejects an ID substitution returned by plan, reserve, or initialize. Before reserving anything, it also requires bothinitialize.requiredandinitialize.recommendedto be present booleans; a partial or malformed decision fails before mutation.Every valid plan reaches the reservation endpoint, including static and other reserve-only plans. The CLI forwards the plan settings to reserve and calls initialize only when the authoritative plan requests it, so recovery cannot turn a reserve-only plan into an initialized app.
Interrupted creates remain resumable without weakening the server create boundary. If reserve fails or returns an unreadable response, the CLI performs an authenticated lookup for the exact app. It resumes only when the response explicitly proves a matching caller-owned, revision-1 idle reservation with the same name, environment, persistence, and runtime class, no active or deleted state, and a present, well-typed, empty versions array. Other-owner, publisher-only, malformed, mismatched, deleted, or established responses retain the original collision failure.
For dynamic apps, the companion Compose change keeps initialization required in the plan until the server records the runner variant. That server-authoritative marker lets a retry resume initialization after a partial failure without causing an intentional reserve-only app to initialize.
If the reservation result and follow-up inspection are both inconclusive, the CLI returns
reservation_outcome_unknownwith the exactbb apps getcommand to inspect the app. Initialization failures explain that the ID is already reserved and that repeating the same create command will reconcile and continue.squareup/blockplat-compose#259 supplies the authoritative create-only endpoint, collision semantics, and initialization-state plan behavior. Deploy that server change first, then include this client change in a subsequent
bbrelease.Related issue
N/A.
Testing
cd bb-cli && source ../bin/activate-hermit && just check(166 unit tests, 81bbend-to-end tests, 22 CLI end-to-end tests, and 6 acceptance-contract tests)fmt-check,tauri-check,clippy, andcheck