From 59ee3476514417dea6bfaf50571e0ee52671ce91 Mon Sep 17 00:00:00 2001 From: Ronen Mars Date: Sun, 2 Aug 2026 00:17:22 +0300 Subject: [PATCH 1/2] docs(landing): correct the runbook against what the real run proved [skip-ci] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The landing executed on 2026-08-01 as #497, #498 and #499 — all three slices merged with zero conflicts, and the final tree comparison came back with nine differing files, every one EXPECTED. This records that outcome and fixes the three claims the run disproved. Guard B's enumeration must not be narrowed to `$FORK..origin/main`. Seven of the eight hand-listed commits are ancestors of the fork, so that range yields 8 files where the curated list yields 21, dropping `package.json`, both workflow files and `KICKOFF-landing-runbook.md` — precisely the regressions `fee27061` exists to undo and that slice A excludes by hand. The reason is structural: prep's history is older than the fork, so a slice can revert `main` work that landed before the divergence point, and any enumeration bounded below by the fork is blind to that class. Nothing slipped through on the run, and it was checked rather than assumed — the wider 21-file guard was re-run retroactively across the whole landing and every removed line is a genuine supersession. PR #457 does not close itself. A PR's diff is computed against the merge-base, and landing content by replay never moves it, so #457 sat open reporting 73 changed files after its content had fully landed. It has to be closed by hand once the tip-to-tip diff is clean. Adds a rule that any `--delete-branch` needs its dependents enumerated first, open PRs that declare the branch as their base included, not only things downstream of it in ancestry. This run hit the harmless ancestry case when `land/slice-a` was deleted before slice B needed it as a rebase base. The streamer landing hit the base-ref case an hour earlier: GitHub closed the dependent PR silently, its content dropped out of that landing, and the automation reported success. Also records that the version-conflict path is still unexercised — `main` was ahead at 188/40 but no slice touched either version file — and clears the `RNSentry` note that #495 resolved. [skip-ci] --- LANDING-integration-to-main.md | 179 +++++++++++++++++++++++++++------ 1 file changed, 150 insertions(+), 29 deletions(-) diff --git a/LANDING-integration-to-main.md b/LANDING-integration-to-main.md index 69d3a3c0..25135fb4 100644 --- a/LANDING-integration-to-main.md +++ b/LANDING-integration-to-main.md @@ -1,7 +1,18 @@ # Landing `land/integration-prep` onto `main` in reviewable slices -> **Status:** Steps 0, 1 and 1a are done. Audited and rehearsed end-to-end on 2026-08-01; the -> plan below is the rehearsed one, not the original. +> **Status: EXECUTED. All three slices are on `main`.** Landed 2026-08-01 as `#497` (slice A, +> merged `8eeaa823`), `#498` (slice B, 9 commits `aaef4af7`…`dcc8c096`) and `#499` (slice C, 13 +> commits). Zero conflicts in all three. The final tree comparison against the frozen prep +> snapshot came back with nine differing files, every one classified EXPECTED — no drift, nothing +> unexplained. The run log, with the full conflict ledger and per-slice verification output, is at +> `../tb-mobile-landing-run/RUN-LOG.md`. +> +> What follows is now a **record of a completed operation**, kept because the same shape recurs. +> Sections below written in the imperative are the procedure as executed, corrected where the run +> proved them wrong. `land/integration-prep`, `backup/prep-landing-2026-08-01` and the +> `archive/prep-*` tags are retained deliberately — after `9cf00d99`, the branch is the only +> remaining witness to what this content was supposed to be. +> > **Do not trust any commit count or SHA written here.** Every number moves — `main` takes a > version bump on every ship, and prep is still a live merge target. Re-measure immediately > before acting: @@ -186,11 +197,29 @@ are the historical bulk and are handled by the slices below, not by the loop. ### PR #457 `#457` is `land/integration-prep → main` — **the integration branch's own PR, not an -alternative to slicing.** As each slice merges its diff shrinks; when the last slice lands it is -empty and closes itself. **Do not merge it whole mid-slicing** — that lands every remaining +alternative to slicing.** **Do not merge it whole mid-slicing** — that lands every remaining slice in one opaque commit and discards exactly the bisectability this operation exists to recover. +> **Correction — it does not close itself.** An earlier revision said "when the last slice lands +> it is empty and closes itself". That is wrong, and the real run proved it: after all three +> slices merged, `#457` was still OPEN and still reporting **73 changed files, +3446 / −1126**, +> while the actual tip-to-tip tree diff had drained to nine EXPECTED files. +> +> **A PR's diff is computed against the merge-base, not tip-to-tip.** Every prep commit landed on +> `main` under a *new* SHA — slice A as a fresh cherry-pick commit, B and C as rebased replays — +> so nothing prep points at became reachable from `main`. The merge-base never moved, and GitHub +> keeps rendering the entire historical divergence. It will sit there looking like unlanded work +> forever. +> +> **Close it by hand once the last slice merges**, after confirming the tip-to-tip diff is clean: +> ```bash +> $G diff --stat origin/main origin/land/integration-prep # expect only the EXPECTED set +> gh pr close 457 +> ``` +> This generalises: any PR whose content is landed by *replay* rather than by merge must be +> closed manually. Self-closing only happens when the head commits themselves become reachable. + --- ## Step 1 — One-time prep rebase — **DONE** @@ -294,18 +323,56 @@ proven otherwise. **Regenerate the list before each slice — but membership is not the check.** The list grows when `main` touches a file it has not touched before; it stays the same size when `main` touches one -already on it, and that second case is the dangerous one, because a regenerated-but-identical -list reads as "nothing changed" when the baseline moved underneath it. Always re-diff against -`main`'s *current* content, never against the list alone. - -A live instance, as of 2026-08-01: PR `#495` -(`fix(ios): add RNSentry to the path-dependent Podfile.lock checksum list`) touches `CLAUDE.md`, -`scripts/reset-podfile-lock-path-noise.sh` and its test. All three are already on the 21-file -list, so the list is still 21 entries after it merges — and yet `CLAUDE.md` is touched by -`309bd80e` in slice B and by `f3487f97` in slice C. Both slices therefore carry a copy of -`CLAUDE.md` that predates `#495`, and Guard B is the only thing standing between that and a -silent revert of the fix. Rebase both onto `main` after `#495` lands and re-run Guard B against -the new content. +already on it, and that second case *looks* dangerous, because a regenerated-but-identical list +reads as "nothing changed" when the baseline moved underneath it. Always re-diff against `main`'s +*current* content, never against the list alone. + +### The enumeration must reach back past the fork — do not narrow it + +> **Correction, and the most important one this run produced.** A companion document rewrote the +> enumeration above as: +> ```bash +> $G log --format='' --name-only --no-merges $FORK..origin/main | sort -u +> ``` +> and expected it to reproduce the 21-file list. **It does not — it yields 8** — and the +> difference is not cosmetic. Seven of the eight commits hand-listed above are **ancestors of +> `$FORK`**, so a `$FORK..origin/main` range excludes them by construction. + +The 15 files that narrower range drops include `package.json`, `.github/workflows/test.yml`, +`.github/workflows/deploy.yml` and `KICKOFF-landing-runbook.md` — **which are precisely the +regressions `fee27061` exists to undo and that slice A excludes by hand.** The guard that is +supposed to catch that class of mistake would have been blind to all of it. + +The reason is structural and worth stating once: **prep's history is older than the fork.** A +slice can therefore revert `main` work that landed *before* the divergence point, not only after +it. Any enumeration bounded below by `$FORK` cannot see that class. Enumerate `main`'s +content-bearing commits directly, as the `for c in …` loop above does, and accept that the list +must be curated rather than derived from a range. + +**Nothing slipped through on the real run**, but only by luck of composition, and it was checked +rather than assumed. Retroactively re-running the wider 21-file guard across the whole landing +(`$G diff origin/main -- `) shows every removed line is a +genuine supersession: `package.json` drops `expo-mcp` (`#489`) and rewrites `test:e2e:mock` +(`#470`), the ship scripts move their `SENTRY_RELEASE` exports, `docs/sentry-releases-investigation.md` +loses the real org names by design, and this document supersedes its own earlier text. +`.github/workflows/deploy.yml`, `CLAUDE.md` and `docs/troubleshooting.md` are additions only. +**No file on `main` was reverted.** + +### What actually happened to `CLAUDE.md` + +`#495` (`fix(ios): add RNSentry to the path-dependent Podfile.lock checksum list`) touches +`CLAUDE.md`, `scripts/reset-podfile-lock-path-noise.sh` and its test, and landed on `main` after +prep had already been cut. `CLAUDE.md` is touched by `309bd80e` in slice B and by `f3487f97` in +slice C, so **both slices carried a copy predating `#495`**. + +Guard B was run against `main`'s current content on both, and both came back **additions only** — +`+16 / −0` on slice B, `+10 / −0` on slice C — with `#495`'s four-checksum paragraph intact. + +**The mechanism, which was likely but never guaranteed: `git rebase` applies patches, not whole +files.** `309bd80e` adds a "Worktrees" section and `f3487f97` adds an "Expo MCP" section; neither +hunk overlaps `#495`'s, so all three compose. Had any of them edited the same paragraph, the +replay would have carried the stale text and the guard is the only thing that would have caught +it. Run it regardless — a guard that only fires when you already suspect trouble is not a guard. --- @@ -459,6 +526,41 @@ rebase and was only removed once the flag was added. Merge strictly in order, one at a time, waiting for green — per `CLAUDE.md` → "One PR at a time". +### Before any `--delete-branch`, enumerate what depends on that ref + +**Rule: before running `gh pr merge --delete-branch` (or deleting any branch on `origin`), list +every dependent of that ref — open PRs that use it as a *base branch* included, not only things +downstream of it in ancestry.** + +Two kinds of dependency look alike and behave nothing alike: + +- **Ancestry-stacked** — a later branch was cut from this one. Deleting the ref is harmless; the + commits are reachable from wherever they landed. Re-point the rebase at the merge target: + ```bash + $G rebase --no-keep-empty --onto origin/main + ``` +- **Base-ref-stacked** — an *open PR* declares this branch as its base. **Deleting the ref makes + GitHub close that PR automatically**, and the closure reads like ordinary cleanup in the log. + +This run hit the benign kind: `gh pr merge --delete-branch` removed `land/slice-a` before slice B +needed it as a rebase base. `--onto origin/main $A_END` is the same operation, since the merged +trunk and the deleted branch had identical trees, and nothing was lost. + +**The same GitHub behaviour is not benign in the other case, and it has already cost real work.** +An hour earlier, the equivalent landing on the streamer repo deleted a branch that an open PR +declared as its base. GitHub silently closed that PR, its content dropped out of the landing +entirely, and the automation reported success — the failure produced no error, only a missing +PR nobody was looking for. Check first: + +```bash +gh pr list --state open --json number,title,baseRefName \ + --jq '.[] | select(.baseRefName == "")' +``` + +If that returns anything, re-target those PRs (`gh pr edit --base main`) **before** merging +the branch away. An empty result is what makes `--delete-branch` safe, and it costs one command +to establish. + --- ## Verification @@ -528,14 +630,15 @@ The first three are exactly `#481`, which `main` has and prep never absorbed. Th | Risk | Mitigation | |---|---| | A slice reverts `main`'s post-divergence work | Guard A + Guard B, every slice, no exceptions. This is what `fee27061` cost. | -| Version regression on `app.json` / `build.gradle` | Take `main`'s (higher) values. Read the live numbers — they moved seven times in the last week. Both branches happened to sit at 187/39 during the rehearsal, so this was *not* exercised. | +| Version regression on `app.json` / `build.gradle` | Take `main`'s (higher) values. Read the live numbers — they moved seven times in the last week. **Still unexercised.** Both branches sat at 187/39 through the rehearsal, and on the real run `main` was ahead at 188/40 but *no slice touched either file*, so no conflict arose. This path has never been tested and must not be treated as proven. | | Squash-vs-drop confusion loses content | Two pairs are patch-identical; everything else is `fixup`. Never `drop`. | | Plain `rebase ` replays the wrong range | Always `rebase --no-keep-empty --onto `. | | An already-empty commit survives the rebase | `--no-keep-empty`. | | `land/integration-prep` drifts | The snapshot is the freeze (Step 0b). Cut slices from the snapshot, never the live branch. | -| PR #457 merged whole by accident | It drains and closes itself. Never merge it during slicing. | +| PR #457 merged whole by accident | Never merge it during slicing. It does **not** close itself — its diff is computed against a merge-base that replay never moves. Close it by hand after the last slice. | +| A branch deleted out from under an open PR that bases on it | Enumerate base-ref dependents before every `--delete-branch`. GitHub closes such PRs silently; this already destroyed a PR's content on the streamer landing. | | A slice is red despite local checks | Fix forward inside that slice; never merge red. | -| `ios/Podfile.lock` churn | Always `bundle exec pod install`, then `scripts/reset-podfile-lock-path-noise.sh`. **Note the script's `NOISE` regex does not cover `RNSentry`,** which drifts the same way at an unchanged version. | +| `ios/Podfile.lock` churn | Always `bundle exec pod install`, then `scripts/reset-podfile-lock-path-noise.sh`. The gap this row used to flag — the `NOISE` regex missing `RNSentry` — was closed by `#495`; all four path-dependent checksums are covered on `main` now. | | A batch Jest failure mistaken for a defect | Re-run the suite alone. `sessionNames.test.ts` failed in batch and passed in isolation during the rehearsal, exactly like the documented `SessionScreen.*` suites. | **Rollback:** nothing is destructive until a slice merges. `land/*` branches are scratch; @@ -560,13 +663,31 @@ them shipping a regression, is not a trade worth making. --- -## Rehearsal - -The full landing was rehearsed locally on 2026-08-01 against this three-slice plan: all three -slices applied, **zero conflicts**, all five checks green per slice, and the final tree matching -the prep snapshot except for `#481` and `Podfile.lock` noise. - -The rehearsal notes — provenance SHAs, the conflict ledger, verbatim verification output, the -B4 classification and the exact ordered replay script for `origin` — live outside the repo at -`../tb-mobile-landing-rehearsal/REHEARSAL-NOTES.md`, alongside the audit that produced this -plan in `PHASE-A-REPORT.md`. +## Rehearsal, then the real run + +The landing was rehearsed locally on 2026-08-01 against this three-slice plan: all three slices +applied, **zero conflicts**, all five checks green per slice, and the final tree matching the prep +snapshot except for `#481` and `Podfile.lock` noise. The rehearsal notes — provenance SHAs, the +conflict ledger, verbatim verification output, the B4 classification and the exact ordered replay +script for `origin` — are committed at +[`docs/landing/2026-08-01-rehearsal-notes.md`](docs/landing/2026-08-01-rehearsal-notes.md), +alongside the prompts that produced them in the same directory. + +**The real run then reproduced the rehearsal almost exactly.** Same boundary SHAs, same slice A +file list, same 10-requested / 9-applied on slice B, same `fee27061` self-neutralisation from five +files to two, and the same verification counts to the test (956 / 255, 957 / 255, 960 / 261). +Slice C ran 13 commits rather than 11 because `#494` and `#496` reached prep after the rehearsal +ended; landing the documents that describe the landing was a deliberate choice, not an oversight. + +Where the real run diverged, it diverged in what the *documents* claimed rather than in what the +history did — the Guard B enumeration, `#457`'s self-closing, and the `--delete-branch` +dependency check, all corrected above. The run log with the full ledger is at +`../tb-mobile-landing-run/RUN-LOG.md`. + +### Still outstanding + +One rehearsal correction remains unapplied: `sessionNames.test.ts` should be added to the +load-sensitive suite list in [`docs/troubleshooting.md`](docs/troubleshooting.md) → "Jest test +suites". It failed in batch and passed in isolation during the rehearsal, exactly like the +`SessionScreen.*` suites already documented, and it did not recur on the real run. That file lives +on `main` and needs its own one-line PR against `main` — it is deliberately not bundled here. From 78b7be50715d202c3e473aa0d1815f296985fa9f Mon Sep 17 00:00:00 2001 From: Ronen Mars Date: Sun, 2 Aug 2026 00:43:17 +0300 Subject: [PATCH 2/2] docs(landing): record the on-device verification traps the real run hit [skip-ci] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The end-to-end check passed from `main` on a physical iPhone 17 Pro, but only after three false starts the Verification section did not warn about. A bare `npx expo run:ios --device` cannot sign this app on a physical device. Xcode's automatic signing regenerates its own Team Provisioning Profile every build and ignores hand-made ones, so the profile never carries App Groups and the build dies during "Planning build" with six errors and xcodebuild exit 65. That is exactly the failure #480 exists to solve, and it ships `scripts/dev-device.sh` for it — the six errors mean the wrong command was used, not that signing regressed. The existing block is right for a simulator, and only its `` placeholder said so. The other two faults arrived together and are independent, which matters because either can occur alone. The first is that a Metro already listening on 8081 captures the device. `expo run:ios` is non-interactive here, so it answers its own port-conflict prompt with `Skipping dev server`, and the device attaches to whatever already owns the port — on this run a server rooted at the repo root on `land/integration-prep` rather than the worktree under test. This is the dangerous one precisely because it is silent: with a healthy bundler it produces a working app showing another branch's code, and the log reports success. The remedy is to check the port owner before trusting the result and to confirm the bundle root in the Metro log. The second is that a long-lived Metro's resolver cache goes stale when its tree is rewritten underneath it. That server had been running for eight hours across the whole landing and could no longer resolve `react/jsx-runtime`, which was present on disk in both trees the entire time. This has nothing to do with which branch is served — it reproduces on the correct branch, with no mismatch to point at — so it is documented as its own failure with `--clear` as the fix. Both were visible in one red screen, and only the module path in the error revealed that the tree was also wrong. Recording them as one story would send the next reader hunting a branch-contamination bug that does not exist. [skip-ci] --- LANDING-integration-to-main.md | 48 ++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/LANDING-integration-to-main.md b/LANDING-integration-to-main.md index 25135fb4..e68d8f5e 100644 --- a/LANDING-integration-to-main.md +++ b/LANDING-integration-to-main.md @@ -600,6 +600,54 @@ npx expo run:ios --device "" Expect the session hub to render with server groups, session cards and conversation rows. +#### On a physical device, that command cannot sign — use `dev:device` + +> **Correction.** The block above is correct for a **simulator** only, and the `` +> placeholder is the only thing that says so. On a physical device a bare `npx expo run:ios +> --device` fails during "Planning build" with six App Groups errors and `xcodebuild` exit 65 — +> **by design**, not as a regression: +> ``` +> Provisioning Profile "iOS Team Provisioning Profile: com.ronenmars.threadbase" +> does not support the App Groups capability. +> ❌ Threadbase → group.com.ronenmars.threadbase +> ❌ ExpoWidgetsTarget → group.com.ronenmars.threadbase +> ``` +> Xcode's automatic signing regenerates its own Team Provisioning Profile on every build and +> ignores hand-made ones, so the profile it produces never carries App Groups. `#480` exists +> precisely to solve this and ships `scripts/dev-device.sh`, which discovers an installed +> development profile per target and injects manual signing through `XCODE_XCCONFIG_FILE`: +> ```bash +> DEVICE_UDID= npm run dev:device +> ``` +> `#480` deliberately leaves Debug on `CODE_SIGN_STYLE = Automatic` in the committed project, so +> an unset UUID behaves exactly as before and simulator builds, fresh clones and CI are all +> unaffected. Seeing those six errors means the wrong command was used, not that signing broke. + +**Prefer a physical device for this check.** The landing carries `#480` (App Groups signing for +on-device Debug builds), the Live Activity work and the widget target; a simulator exercises none +of them. + +#### Serve the bundle from the tree under test, or the check proves nothing + +**A Metro already listening on 8081 silently invalidates this whole step.** `expo run:ios` is +non-interactive here, so it answers its own "use port 8082 instead?" prompt with `Skipping dev +server` and the device attaches to whatever is already on 8081. If that is a checkout of a +*different* branch, the app renders that branch's JS while the log reports success. + +Verified on 2026-08-01: the device attached to a Metro rooted at the repo root on +`land/integration-prep` and red-screened with a module path that gave it away — +`Unable to resolve module react/jsx-runtime from /Users/ronenmars/dev/ai-tools/tb-mobile/app/session/[id].tsx`. +That server had been running for eight hours while its tree was rewritten underneath it, so its +resolver cache was stale; `react/jsx-runtime.js` was present on disk the whole time. + +Check the owner before trusting the result, and confirm the bundle came from the right root: + +```bash +lsof -nP -iTCP:8081 -sTCP:LISTEN # who owns the port +npx expo start --dev-client --clear # from the worktree under test +# then confirm in the Metro log: iOS Bundled …ms node_modules/expo-router/entry.js +``` + ### The final check is a tree diff, not a commit count > **The old `0 0` target is unreachable and has been removed.** It read