mirror of
https://github.com/block/buzz.git
synced 2026-11-05 00:41:28 +00:00
b9edbbceaf
Carl's review of PR 6715 found two holes. The UI change was correct, but the native side did not hold the two invariants that the UI depends on. This commit closes both, and it closes the two test gaps the review named. P1 — an update must not report success while the delete guard can refuse. `commit_team_update` saved `teams.json` first, then propagated the roster to the agent store best-effort. A lost agent write left the team empty on disk while an agent kept the team id. `delete_team_with_cascade` then refuses the team. That is the exact empty-and-undeletable state this feature exists to remove, and the command reported success for it. A second save did not repair it. `apply_team_membership_delta` reads the previous-to-current delta, and after the first save both are empty, so the delta does nothing. Only boot repair could clear the binding, which needs an app restart. Two changes: - `detach_agents_outside_roster` reconciles against the *current* roster instead of a delta. An instance bound to this team whose persona is not on the roster loses the binding. The invariant is state-based, so a second save repairs a lost detach. Bindings to other teams are untouched. - `propagate_membership` returns the error. `commit_team_update` now maps it to a message that names the effect and the action. `create` keeps the best-effort policy through `propagate_membership_best_effort`, which is now a wrapper: a create has no id yet, so a retry can mint a duplicate team, and a missing backfill blocks nothing. P2 — the producer must refuse a snapshot of a team with no members. `build_team_export_snapshot` mapped an empty roster to `members = []`, and the importer rejects that artifact. The disabled menu item is not an integrity boundary: both export commands stay callable, and a share dialog can submit after the roster becomes empty, because the command reads the team by id at the time of the call. The guard is now in the producer, where both commands pass. Tests. - `update_never_reports_success_while_the_delete_guard_sees_the_agent` is the command-level regression the review asked for. It starts with a bound agent, empties the roster, and applies the real `agents_referencing_team` predicate to the store the command left. It pins the report on failure and the repair on retry. - `agents_referencing_team` becomes `pub(crate)` so that test can use the production predicate instead of a copy. - `empty_team_export_is_refused_before_any_bytes` holds the P2 guard. The importer side is already pinned by `validate_rejects_zero_members`. - Three unit tests cover the reconcile: the strict error path, the recovery save, and the scope of the reconcile. - `commit_returns_ok_when_agent_save_fails` becomes `commit_create_returns_ok_when_agent_save_fails`. Its update half is removed, because that contract changed on purpose. The e2e mock did not hold either contract, so no e2e could cover them. `e2eBridge.ts` now carries `team_id` on a mock agent, detaches on update like the native propagate, and refuses a delete while an agent still references the team. The `a team can be emptied and then deleted` spec seeds a bound agent, and it asserts the delete is refused first. Gates: cargo test 2871 pass; clippy -D warnings clean; cargo fmt clean; tsc clean; node tests 5401 pass; Playwright integration 188 pass. The 4 remaining integration failures need a local relay on port 3000 and are not related to this change. Signed-off-by: Fizz <550f2cb2562fc05f87c4839dc3c9b1bfca4d68db4183f8b54d3d477f6836a91d@buzz.block.builderlab.xyz> Co-authored-by: Storme Briscoe <storme@squareup.com> Signed-off-by: Storme Briscoe <storme@squareup.com>