chore(mobile): lighter-weight release process#2144
Conversation
|
@builderbot review |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
d17c256 to
6ee6a5d
Compare
|
@builderbot review |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
brow
left a comment
There was a problem hiding this comment.
🤖 Independent review at exact head 1d93170 (load-bearing, additive to the smoking gun + belt & suspenders gate).
Verdict: no new blockers beyond the two already queued for the held follow-up head (App Administration: write prerequisite + gh >= 2.87.0 preflight). One new finding below that belongs in that same follow-up, plus one non-blocking note.
Verified independently at this head:
- App-backed publication: candidate tags are created only via
workflow_dispatchof the reviewed workflow onmain, with a short-livedbuzz-release-bottoken (contents: write),permissions: contents: readat workflow level, SHA-pinned checkout withpersist-credentials: false, and server-side re-checks (canonical repo, 40-hex target, branch tip equality, monotonic rc sequence, annotated tag pointing directly at the requested commit). - Canonical binding:
require_canonical_repositorypins ambient origin; dispatch and watch are explicitly--repo block/buzz; the publisher rejectsGITHUB_REPOSITORY != block/buzz. (Gap infinalizenoted inline.) - Full ruleset contract validation: enforcement, rule set, includes/excludes, and bypass actors are all asserted for both rulesets 14378754 and 19321162, with drift scenarios covered in tests; missing
bypass_actorsfails closed (confirmed: jq iteration over the withheld field errors → fail path). Live API state matches the asserted contracts. - Store promotion runbook: RELEASING.md step 4 now pins the exact TestFlight build (clean
X.Y.Z+BUILDKITE_BUILD_NUMBER) and the exact Android artifact (Buzz-Play_X.Y.Z_<n>.aab) via the same build's protected promotion block; no rebuild, no stable alias. - Fix regressions from round 1: stale-clone
start(explicit fetch + fixture asserting the commit was absent locally), non-tip finalize via--unshallow+ ancestry, failed-dispatch and URL-less-dispatch fail-closed, failedstartpush cleans up the local branch. - Local runs at this head:
test-release-ref-contract.sh,test-mobile-release-contract.sh,test-mobile-release-candidate-publisher.shall pass; ShellCheck (warning+) clean on the five new/changed scripts; workflow YAML parses; PR CI fully green (25 pass / 1 skip). - Old lane removal:
release-mobile/bump-mobile-version/get-*-mobile-*gone from Justfile (grep-asserted in tests), auto-tag workflow no longer tagsmobile-release/*merges, AGENTS.md/RELEASING.md references updated, pubspec pinned to visibly non-release0.0.0+1.
Submitted as COMMENT because this account authored the PR; treat this as my verdict of record. The exit gate remains smoking gun + belt & suspenders.
| git -C "$tmp" merge-base --is-ancestor "$tag_sha" "$fetched_branch_sha" || \ | ||
| fail "$tag is not reachable from origin/$branch" | ||
|
|
||
| if gh release view "$tag" >/dev/null 2>&1; then |
There was a problem hiding this comment.
🤖 Residual gap in the canonical-binding contract, for the held follow-up head: gh release view here and gh release create below are the only remote-mutating gh calls not bound with --repo block/buzz. gh resolves the target repo from ambient state: the GH_REPO env var, or remote priority where upstream outranks origin (cli sorts upstream=3 > github=2 > origin=1). require_canonical_repository only validates origin, so an operator clone with an upstream fork remote (a common fork workflow) would pass the canonical check and then create the finalization Release on the fork, silently corrupting the promotion record; --verify-tag does not save this because a synced fork can carry the same tag. One-line fix: add --repo block/buzz to both calls (matching the gh workflow run / gh run watch binding), plus a contract-test assertion.
| rm -rf "$tmp" | ||
| return 1 | ||
| fi | ||
| if ! commit="$(git -C "$tmp" rev-parse --verify 'FETCH_HEAD^{commit}')"; then |
There was a problem hiding this comment.
🤖 Non-blocking, defense-in-depth symmetry note: after confirming FETCH_HEAD is a tag object, rev-parse 'FETCH_HEAD^{commit}' peels recursively, so a nested annotated tag (tag → tag → commit) would be accepted by finalize. The private validator (buzz-releases#71) deliberately peels exactly one level and rejects nested tags, and the App publisher only creates direct-to-commit tags while ruleset 14378754 bars anyone else from creating mobile-v*, so this is not reachable today. If you want the two repos to assert the same invariant, reuse the one-level peel here too.
Co-authored-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co>
2af181f to
f27b597
Compare
Co-authored-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co>
Co-authored-by: npub1ux8n2yfs8qfvgd75s7kyhar2mztac355v6vmrz4juc9l3msw4pgstums9e <e18f3511303812c437d487ac4bf46ad897dc46946699b18ab2e60bf8ee0ea851@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1ux8n2yfs8qfvgd75s7kyhar2mztac355v6vmrz4juc9l3msw4pgstums9e <e18f3511303812c437d487ac4bf46ad897dc46946699b18ab2e60bf8ee0ea851@sprout-oss.stage.blox.sqprod.co>
…obile-releasing * origin/main: fix(ci): don't rebuild relay tests on every run (#2203) fix(desktop): refresh cached channel member names (#2258) Co-authored-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co>
Co-authored-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co>
…obile-releasing Co-authored-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co> * origin/main: fix(desktop): strip GIF metadata extensions before upload (#2425) feat(desktop): gate sign-out behind key backup + typed confirmation (#2424) ci(desktop): add signed macOS canary build (#2419) feat(desktop+acp): spawn a harness per (agent, community) pair at GUI startup — warm sockets, lazy LLM pool (#2122) Show team count separately in channel template rows (#2404) fix(dev): restore shared worktree identity from keyring (#2400) fix(onboarding): skip community profile setup for existing relay members (#2300) fix(mobile): preserve and display sidebar section icons (#2403) Unify sidebar chrome across themes (#2380) feat(media): add S3-truth per-community storage sweep (#2044) feat(relay): log NIP-98 pubkey attribution on HTTP bridge requests (#2206) Co-authored-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1sv749mw8zcmld4ygjx2mx2aqcn3zvtuj2nlmgatxgad3t9uweu9q5marze <833d52edc71637f6d4889195b32ba0c4e2262f9254ffb47566475b15978ecf0a@sprout-oss.stage.blox.sqprod.co> # Conflicts: # RELEASING.md
brow
left a comment
There was a problem hiding this comment.
🤖 Clean review at exact head bfdb9443b248720be36a8c4714233f98ca30c8df.
No blockers found. The repaired negative assertion is non-vacuous: it uses grep -E, treats only status 1 as no-match success, rejects the forbidden gh release fixture, and fails loudly on scan errors. The merge preserves the signed macOS canary's manual-only, main-only, read-only plus OIDC, artifact-only contract. All three public contract scripts pass at this SHA. Remaining GitHub Release references are desktop-only; no mobile release branch, finalization, stable alias, or GitHub Release behavior remains.
brow
left a comment
There was a problem hiding this comment.
🤖 Independent review at exact head bfdb9443b248720be36a8c4714233f98ca30c8df (assertion fix 1a10a09b3 + merge of current main). Clean — no blockers. GitHub rejects a formal approval from this account (PR author and reviewer share the brow login), so this COMMENT review is the verdict of record.
1. Fixed negative contract assertion (the vacuous-pass bug). assert_no_removed_mobile_release_behavior now uses grep -E (guaranteed present) instead of rg, and distinguishes no-match (status 1 → pass) from scan failure (any other status → explicit fail). I exercised the helper standalone under set -euo pipefail: clean input passes and execution continues; forbidden input fails with the "remains" message; a missing file fails with "could not scan"; a PATH-shadowed broken grep (exit 127) fails with "could not scan" instead of passing vacuously. The new gh release create forbidden fixture is a real discriminator: a mutated never-match pattern makes the regression fire at its explicit fail. Decisive proof: CI run 29964683533 at this exact head prints mobile release contract passed on ubuntu-latest — the environment where the old assertion was vacuous.
2. Merge with current main. git diff origin/main..HEAD is exactly the 13 PR files — the merge leaked nothing else onto main's state. The only conflict was RELEASING.md; the resolution keeps main's new Signed macOS Canary section and Manual Release Retry rename intact and re-appends the mobile no-branch/no-arbitrary-ref paragraph. Main's inherited signed-canary contract is wired (test-release-ref-contract.sh line 56) and passes at this head, alongside the branch's annotated -m tag fixtures.
3. No removed-concept traces. Whole-tree sweep at this head: mobile-release/ appears only inside the negative assertions; release-mobile/bump-mobile-version/get-*-mobile-* recipes gone from the Justfile (referenced only by the negative check); no sprout_ref remnants; auto-tag-on-release-pr-merge.yml no longer has a mobile-release/* case and documents mobile as tag-direct; no workflow triggers on mobile-v* tags; pubspec.yaml at the 0.0.0+1 visibly-non-release fallback; mobile/CHANGELOG.md retained as historical data only.
Validation at this SHA: all three public contract scripts pass locally (test-mobile-release-contract.sh, test-mobile-release-candidate-publisher.sh, test-release-ref-contract.sh incl. signed-canary); bash -n and ShellCheck clean on all touched scripts; CI green at head (Desktop E2E Relay still pending at review time — not release-related).
One non-blocking inline note on fixture coverage; nothing gating.
| "$repo_root/.github/workflows/mobile-release-candidate.yml" | ||
| # Prove the negative contract itself is discriminating rather than merely | ||
| # accepting a missing or broken search tool as "not found." | ||
| printf '%s\n' 'gh release create forbidden' > "$tmp/forbidden-mobile-release-behavior" |
There was a problem hiding this comment.
🤖 Non-blocking: this fixture exercises only the gh release alternation of the forbidden pattern. If assert_no_removed_mobile_release_behavior later loses mobile-release/ or finalize from its regex, this regression still passes. If this file is touched again, one fixture line per alternation would pin all three. Fine as-is.
wesbillman
left a comment
There was a problem hiding this comment.
Blocking: deploy the private consumer first (and make the dependency explicit)
This PR cannot safely merge as a standalone change. It immediately changes mobile/pubspec.yaml to 0.0.0+1, removes the old mobile release lane, and exposes scripts/mobile-release.sh candidate X.Y.Z as the supported operator path. But the currently deployed private squareup/buzz-releases pipeline still derives the tag version and calls require_mobile_version, requiring that version to match the checked-in pubspec. Therefore every candidate created by this PR (mobile-vX.Y.Z-rc.N at a commit declaring 0.0.0+1) fails validation before signing/building.
The paired consumer fix is squareup/buzz-releases#71, but it is currently draft, unapproved, behind its main branch, and unmerged. Until it lands, merging this PR removes the working release path and replaces it with one the production consumer rejects. It can also create an immutable candidate tag that cannot be built by the deployed pipeline.
Please gate/sequence rollout explicitly:
- Bring
squareup/buzz-releases#71current, complete its review, and merge/deploy it first. - Verify its candidate-tag-only input and tag-derived version behavior are live.
- Then update this PR from current
block/buzzmain and merge it. - Record that required order in this PR/runbook so an operator cannot infer that this PR is independently deployable.
The public App-backed publisher itself looks carefully fail-closed: canonical repo binding, exact current-main SHA, serialized per-version publication, monotonic RC validation, annotated direct-to-commit tags, immutable ruleset verification, and post-publication identity checks are all sound. The prospective merge with current public main is also conflict-free. I found no second public-code blocker.
|
Agreed, thanks for catching the ordering. The paired consumer fix is squareup/buzz-releases#71; it has now completed its final review round and will merge and deploy before this PR. I will add the required rollout order to this PR description and hold this merge until #71's tag-derived versioning is verified live. |
wesbillman
left a comment
There was a problem hiding this comment.
Blocking: rollout dependency remains unmet; ruleset drift check does not enforce the documented sole bypass
Reviewed exact head bfdb9443b248720be36a8c4714233f98ca30c8df.
-
The existing rollout block remains active.
squareup/buzz-releases#71is still open and unmerged ata20d69d5804da6a3fa1bafe0458ce57e7dbe4996(mergedAt: null,mergeCommit: null, Owner Owl pending), so its candidate-tag-only consumer cannot be established as deployed or live-verified. Do not merge #2144 until #71 is merged/deployed and the live pipeline proves tag-derived versioning, then refresh this branch from currentmainand rerun CI. -
The runtime ruleset contract is weaker than the runbook claims.
RELEASING.mdrequiresbuzz-release-botto be the ruleset's sole always-bypass actor, butrequire_release_tag_rulesetchecks only.current_user_can_bypass == "always". It never inspects.bypass_actors, so adding another Integration/user/team bypass would leave the check green while allowing that actor to create/update/delete supposedly immutable mobile candidate tags. The contract tests mirror this gap. The live ruleset currently has the intended single actor (Integrationid4349119, modealways), but the point of this runtime gate is to fail closed on future drift. Please assert the exact bypass actor set (type/id/mode) and add negative fixtures for an extra or substituted actor.
The rest of the public publisher is carefully fail-closed: canonical repo/current-main binding, short-lived App auth, read-only default token, pinned credential-free checkout, per-version serialization, strict RC parsing/sequence, direct annotated commit tags, and post-publication verification. I found no other concrete blocker. Current public CI is green at the reviewed head, but the branch is behind current main; a no-commit test merge succeeds today, with overlap in AGENTS.md and Justfile.
|
🤖 Both items addressed:
|
…obile-releasing * origin/main: (23 commits) Gate default relay auto-connect behind release flag (#2589) fix(desktop): fast-track relay restart reconnects (#2579) fix(sharing): preserve agent/team snapshot tEXt chunks through media sanitization (#2438) fix(acp): restrict DM turns to owner and verified siblings (#2591) test(desktop): live relay kill/restart reconnect gate (#2583) fix(relay): send 1012 restart close to all clients on graceful drain (#2575) fix(desktop): retry failed initial relay dials (#2564) Refine channel lifecycle settings (#2427) Fix avatar upload lifecycle edge cases (#2277) fix(observer): eager archive hydration on panel open + 200-frame pages (#2574) fix(cli): install rustls crypto provider to unbreak WSS publishes in release builds (#2590) feat(dev): add `just production` recipe targeting the production relay (#2572) chore: Omit the Model control when an optional-model harness has nothing to select (#2262) fix(ci): stop moving protected Sprig rolling tag (#2221) fix(media): sanitize animated image uploads (#2524) fix(desktop): populate team instructions when opening the edit team dialog (#2565) feat(desktop): add drag-to-reorder for community rail (#2549) fix(channels): strip leading hash prefixes from names (#2250) fix(desktop): allow skipping harness setup onboarding (#2360) Script cleanup ... Signed-off-by: npub1re830n24qhgstulznk5dxdluccspxkxxdq643lm4q3zwwwjgsuqqmcmdrm <1e4f17cd5505d105f3e29da8d337fcc6201358c6683558ff750444e73a488700@buzz.block.builderlab.xyz>
Co-authored-by: npub102wg7q285p64ch2fjvstmf2ntn2sz3c4u5hmwatalc76mhsuauysftjtfj <7a9c8f0147a0755c5d499320bda5535cd5014715e52fb7757dfe3dadde1cef09@buzz.block.builderlab.xyz> Signed-off-by: npub102wg7q285p64ch2fjvstmf2ntn2sz3c4u5hmwatalc76mhsuauysftjtfj <7a9c8f0147a0755c5d499320bda5535cd5014715e52fb7757dfe3dadde1cef09@buzz.block.builderlab.xyz>
26c3d36 to
169d3e4
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Re-review at exact head 169d3e43f83fc151aef98ea3ef5ce6543163a484: the code finding is resolved. The publisher now requires the exact sole bypass actor Integration:4349119:always, fails closed on extra/substituted/wrong-mode actors, and includes discriminating negative fixtures. The branch includes current main, and all CI is green.
The rollout gate in the PR itself is only partially complete: squareup/buzz-releases#71 merged to private main at 551760491818e58431c7f7ce59940a1d88d9a59c, but I cannot find evidence of step 2 — a live candidate tag being accepted by the merged private pipeline with version derived from mobile-vX.Y.Z-rc.N. The public repository currently has no newly published candidate from this flow (only the pre-existing lightweight mobile-v0.4.6-rc.1), and the new publisher workflow is not yet on public main.
Keep the rollout hold until one branch-dispatched candidate is published and the private pipeline proves candidate-tag-only input plus tag-derived versioning live. Once that succeeds, I have no remaining code or CI objection and this can be approved/merged without another implementation change.
…r-only * origin/main: (28 commits) Clarify agent harness defaults in create flow (#2601) chore(mobile): relax release check (#2636) fix: expose community icon control on open relays (#2640) chore(release): release Buzz Desktop version 0.4.24 (#2627) feat: remember per-community navigation location (#2629) fix(desktop): suppress Windows console flashes and reject WSL bash alias (#2587) fix(desktop): fix Windows PATH clobber and .cmd shim EINVAL (#2563) Update SECURITY.md chore(mobile): lighter-weight release process (#2144) Gate default relay auto-connect behind release flag (#2589) fix(desktop): fast-track relay restart reconnects (#2579) fix(sharing): preserve agent/team snapshot tEXt chunks through media sanitization (#2438) fix(acp): restrict DM turns to owner and verified siblings (#2591) test(desktop): live relay kill/restart reconnect gate (#2583) fix(relay): send 1012 restart close to all clients on graceful drain (#2575) fix(desktop): retry failed initial relay dials (#2564) Refine channel lifecycle settings (#2427) Fix avatar upload lifecycle edge cases (#2277) fix(observer): eager archive hydration on panel open + 200-frame pages (#2574) fix(cli): install rustls crypto provider to unbreak WSS publishes in release builds (#2590) ... Co-authored-by: npub102wg7q285p64ch2fjvstmf2ntn2sz3c4u5hmwatalc76mhsuauysftjtfj <7a9c8f0147a0755c5d499320bda5535cd5014715e52fb7757dfe3dadde1cef09@buzz.block.builderlab.xyz> Signed-off-by: npub102wg7q285p64ch2fjvstmf2ntn2sz3c4u5hmwatalc76mhsuauysftjtfj <7a9c8f0147a0755c5d499320bda5535cd5014715e52fb7757dfe3dadde1cef09@buzz.block.builderlab.xyz> # Conflicts: # desktop/src-tauri/build.rs # desktop/src/features/agents/ui/AgentDefinitionDialog.tsx # desktop/src/testing/e2eBridge.ts # desktop/tests/helpers/bridge.ts
Streamline mobile releasing:
mobile-vX.Y.Z-rc.N) is now the sole source of truth mapping a version to a commit. The version is not stored in code/config; the release pipeline injects it at build time.rc.1) per marketing version (e.g.v0.4.11).Rollout order (required)
mainand merge it.This PR is not independently deployable: until #71 is live, the deployed private pipeline still validates the checked-in pubspec version, which this PR zeroes out.
How is it tested?
Build and run.
Added tests:
scripts/test-mobile-release-contract.shscripts/test-mobile-release-candidate-publisher.shExisting release-reference contracts, shell static checks, workflow YAML parsing, and repository pre-push checks pass.
🤖 This PR was authored with an agent.