Repository navigation
refactor(schema): stop auto-fencing new tables in desired-state bootstrap - #8620
Merged
Merged
Conversation
Signed-off-by: Nikolaj Goodger <ngoodger@squareup.com>
Signed-off-by: Nikolaj Goodger <ngoodger@squareup.com>
Remove the leftover schema.sql comment for the deleted blanket fence bootstrap, and stop the attach helper's header from telling authors to fence new tables, which contradicts docs/database.md Rule 1. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Nikolaj Goodger <ngoodger@squareup.com>
ngoodger-block
temporarily deployed
to
codex-review
October 27, 2026 08:57 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
github-actions
Bot
added
the
codex-security-review-current
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
Contributor
|
this looks good to me. i'd prefer @TheSentinel454 also review this one since he's been working on db |
TheSentinel454
approved these changes
Oct 27, 2026
TheSentinel454
left a comment
TheSentinel454
left a comment
Contributor
There was a problem hiding this comment.
🤖 LGTM. Checked that every non-excluded community_id table (33) already has an explicit schema/triggers/ attach, so removing the loop drops no existing fence; new test runs and passes in the PostgreSQL Tests lane at this SHA.
ravarora2
added a commit
that referenced
this pull request
Oct 28, 2026
…-handler * origin/main: perf(db,relay): bound thread participant scans and coalesce live summaries (#8635) mobile: find DMs opened ahead of the list everywhere (#8646) BUZZ-413: Add user deletion fence storage (#8632) fix(mobile): open new DMs without waiting for a full channel reload (#8638) Stop requiring the 0032 roster trigger at relay startup (#8611) fix(relay): store a kind:9000 and its membership writes in one transaction (#8606) fix(deps): bump pulldown-cmark to 0.13.5 for RUSTSEC-2026-0414 (#8644) feat(relay): log who and why for NIP-FI shadow would-deny (#8634) feat(mobile): add full-screen profile setup with inline avatar editing (#8628) fix(mobile): preserve invite onboarding and offer identity setup (#8617) refactor(schema): stop auto-fencing new tables in desired-state bootstrap (#8620) Signed-off-by: Ravneet Arora <rarora@squareup.com>
matt2e
added a commit
that referenced
this pull request
Oct 29, 2026
…#8677) ## Summary Fixes two CI flakes from the 21–28 Oct 2026 audit, both in test code with no product change: - The relay E2E helper `collect_until_eose` now drops repeat copies of an event id. - Two Windows shell tests drop a 5 s cap that a cold Git Bash start can exceed. This is one of seven PRs split out of the `flakey-tests` branch (siblings below). ### Related issue None found for these two. The buzz-terminal `passwd_shell()` `getpwuid` race (`env_fence_tests::resolve_shell_falls_through_to_passwd_not_the_default`, run 38590095289) is left to open PR #7687, which already replaces `getpwuid`. #8636 and #7976 edit `buzz-test-client/src/lib.rs`, and #7607 and #7915 edit `buzz-dev-mcp/src/shell.rs`, but none of them changes `collect_until_eose` or these timeouts. ### Flaky tests fixed | Test | Failing runs | Cause | Change | |---|---|---|---| | Relay E2E `e2e_project::test_project_tombstone_deletes_coordinate_and_spares_members` (e2e_project.rs:338, "left: 2 right: 1") | 40408678412#1, job 121879630394 (PR #8620, schema-only) | The EVENT handler sends OK, then fans out in a spawned task. `handle_req` registers the live subscription before its history query, and `req.rs`'s `seen_ids` dedups history rows only. If fan-out runs after the REQ is registered, the event arrives once live and once from history, both before EOSE, and `collect_until_eose` kept every frame. | `collect_until_eose` keeps the first copy of each event id, as production clients do. Its doc comment says a test of the relay's own dedup must read frames with `recv_event`. A new unit test, `collect_until_eose_keeps_one_copy_of_each_event_id`, uses a fake relay that sends the duplicate. | | Windows `buzz-dev-mcp` `shell::tests::basic_echo` (shell.rs:1026, exit 124) | 39675698081#1, job 119485363399 (green on re-run) | 124 is the shell tool's own timeout exit code. `basic_echo` capped the shell at 5 s, and it is one of the first tests in the process to start Git Bash, which can take longer than 5 s on a cold runner. | `timeout_ms: None` (the production 120 s default) in `basic_echo` and in `workdir_is_honored`, which has the same exposure. `timeout_fires` still covers timeouts. | Pre-check: no e2e test relies on the relay's own history dedup. Every exact-count assertion uses a single filter. The NIP-33 "only newest remains" assertions would see a failed replacement as two different ids, and this dedup keeps both. Local evidence is from the original `flakey-tests` commit (fc4518d39): - The new unit test: FAIL before, PASS after. - 13 of the 14 e2e files that call the helper, plus `e2e_thread_roots`, against an isolated local relay. All pass, except `e2e_relay` `test_unarchive_emits_member_added_notification`. That test fails by itself 3/5 with the original helper too, and CI's Relay E2E lane doesn't select it. - The Windows failure can't be reproduced on macOS. The Windows Rust lane is the check. Follow-up (out of scope): make the relay deliver each event id at most once per subscription, keeping the live/history overlap. ### Verification This branch was pushed without local re-verification, by the user's choice: local test runs were overloading the machine and failing for that reason. **Hosted CI is the check.** It was pushed with `git push --no-verify`, because the pre-push hook runs clippy and unit tests. - Both files are byte-identical to their final versions on `flakey-tests` (67185df6a). - Checked for this PR: `git diff --check`, `rustfmt --check`, and a conflict-free merge with all six siblings onto `main`. ### Readiness Test-only change. AGENTS.md's readiness steps (agent review, then human confirmation) are still outstanding, so `buzz-review-completed` is not set. ### Merge order Independent of the other six. It merges cleanly onto `main`, and with every sibling, in any order. ### Sibling PRs Split from `flakey-tests` (one PR per area): #8676 (PostgreSQL lease + lane), #8678 (mobile), #8679 (Tauri apt cache + Windows Rust cap), #8680 (desktop product races), #8681 (desktop E2E state gates), #8682 (desktop E2E composer + row actions). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why?
This is a cleanup that brings the schema bootstrap in line with how the codebase works today, as asked for by
docs/database.md(#8273). Writes are now blocked during community deletion by application admission (begin_community_write_transaction), and that doc's Rule 1 says new tables must not add triggers. Butschema/community_write_fence.sqlstill looped over every table with acommunity_idcolumn and attached the old fence trigger to it. That meant a new table got a trigger on a fresh database and none on a migrated one, so the two schema paths disagreed and new tenant tables were blocked. The doc says this bootstrap change must land "before the feature, or as the feature's first PR."What?
Deletes the blanket fence loop and its
\ifromschema/schema.sql. Existing legacy tables keep their fences through their explicitschema/triggers/files, so no current table loses its fence and runtime behavior is unchanged. Adds a test that a newcommunity_idtable stays unfenced whileeventsandusersstay fenced, and updatesdocs/database.mdandARCHITECTURE.mdto match. It stands on its own; the media-sharing PRs (#8451, #8444) are the first to rely on it.buzz-review-completed