Skip to content

fix(app): the hello is always first, and device removal is confirmed in the future tense - #1983

Merged
AbirAbbas merged 4 commits into
app/v3from
fix/app-hello-first-remove-line
Oct 27, 2026
Merged

AbirAbbas merged 4 commits into
app/v3from
fix/app-hello-first-remove-line

Conversation

@AbirAbbas

Copy link
Copy Markdown
Collaborator

Summary

Two bugs from a post-merge manual check of #1971 and #1977. The check drove the release binary through the desktop's Rust core and rendered the Devices screen.

1. sync.changed could arrive before hello (from #1971). The desktop refuses a connection whose first line isn't hello. One real start failed with first event is "sync.changed", expected "hello".

The backend is built on the appwire server before Serve sends hello. On a slow start, the 30-second idle sweep can run first, and its first sync-status read emits sync.changed.

appwire.Server now holds every frame queued before the hello and writes it right after, in order, so sequence numbers still follow write order. That guarantee holds for any early emitter, not just this one. A server closed without ever saying hello still writes what it held, which keeps TestSequenceNumbersAreConsecutiveAcrossGoroutines' behaviour.

2. The removal confirmation was in the past tense (from #1977). remove_line is shown before the person confirms, but it reused pair.ChatsRevokedLine: "Home was removed — it can no longer sync your chats…".

It now uses pair.ChatsRemoveLine: "Home will no longer sync your chats.", followed by the same warning about what removal can't undo. The devices.remove receipt and the terminal's codeaf devices remove keep the after-removal sentence.

Test plan

  • New TestAnEventEmittedBeforeTheHelloFollowsIt: fails without the fix (line 0 = sync.changed seq 1, want hello seq 1); passes 5/5 under -race with it
  • go test -race ./internal/appwire/: only the two known PROTOCOL.md failures, which are already on the app/v3 baseline list
  • go test ./internal/pair/ ok
  • go test -tags=engine ./cmd/codeaf -run '^(TestAppDevices|TestC[0-9]+App|TestAppNote|TestAnUnknownNote)' ok, including the updated remove_line assertions (it must differ from the receipt)
  • go vet, gofmt clean
  • CI gate (dispatched by hand; PRs into app/v3 do not trigger it)

The desktop side of the duplicated-receipt display is a separate codeaf-desk PR. It works with both the old and the new wording.

🤖 Generated with Claude Code

AbirAbbas and others added 4 commits October 26, 2026 19:16
The desktop refuses a connection whose first line is not the hello. The
backend is built on the server before Serve says hello, and on a slow start
it can emit first: the idle sweep reads sync status every 30 seconds and,
from #1971, emits sync.changed on that first read. A manual run of the
release binary through the desktop core hit "first event is sync.changed,
expected hello" and failed to start.

The server now holds every frame queued before the hello and writes it
straight after, in order, so sequence numbers still follow write order. A
server closed without ever saying hello still writes what it held.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
devices.list's remove_line is shown before the person confirms, but it used
pair.ChatsRevokedLine ("… was removed — it can no longer sync your chats").
remove_line now uses pair.ChatsRemoveLine ("… will no longer sync your
chats") with the same warning about what removal cannot undo; the
devices.remove receipt and the terminal keep the after sentence.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@AbirAbbas

Copy link
Copy Markdown
Collaborator Author

CI evidence, judged against a fresh app/v3 baseline (PRs into app/v3 don't trigger CI, so both were dispatched by hand):

  • Full check on this branch (run 40419272253): 99 failing test names. app/v3 baseline Full at 1e254a7 (run 40419275937): 96 failing.
  • Every failure on this branch except three is also in the baseline. The three are temp-folder cleanup flakes in packages this PR doesn't touch:
    • TestSeniorDevCustomShellModelNeedsKnownOrConfiguredCapacity/missing-window: TempDir RemoveAll cleanup … directory not empty. A different subtest of it flaked the same way on test(app): warm the sync status test's backend before its wire starts #1981's gate.
    • TestTheNodeDoorCommitsARunRowNotesKeyInTheStore (internal/session): TempDir RemoveAll cleanup … .git/worktrees/61: directory not empty.
  • The PR gate (run 40418627054) tested no packages: a hand-dispatched gate only covers what HEAD~1 touched, which here was the change entry. Its only failure is lint in internal/factory/run/chatstage.go (reportCancel possible context leak), which is already failing on app/v3 and isn't touched here.

@AbirAbbas
AbirAbbas merged commit f02b4ca into app/v3 Oct 27, 2026
14 of 20 checks passed
@AbirAbbas
AbirAbbas deleted the fix/app-hello-first-remove-line branch October 27, 2026 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant