2 Commits

Author SHA1 Message Date
jack
0152196ac2
Deflake CI, and make the flake class unrepeatable (#1491)
* Deflake two iOS-sim CI tests with unbounded timing assumptions

Both failed on main-adjacent CI runs during this session's PRs. Neither
was a product bug; both asserted things about real time that a loaded
runner is under no obligation to honour.

**NetworkReachabilityGateTests: a wall-clock upper bound.**
test_monitor_duplicateUpdatesDoNotPostponeOfflineCommit slept 500ms for
real, then asserted total elapsed time was under 1.4s to prove a duplicate
mid-window had not restarted the 1.0s debounce. One CI run took 3.75s. No
wall-clock bound can separate "deadline preserved" from "runner is slow",
because Task.sleep and the asyncAfter flush are both real time and neither
is bounded above.

The deadline property was already covered deterministically one level
down: test_debounce_duplicateObservationsPreservePendingDeadline drives
ReachabilityDebounce with injected timestamps and checks pendingRemaining
directly. So the monitor test now asserts only what needs a real monitor —
that a duplicate still yields exactly one committed false through the
debounce — with an injected clock for the arithmetic and a generous
liveness budget. Renamed to say what it actually checks. No coverage lost,
and it runs in 0.14s instead of ~3.8s because the real sleep is gone.

**NoiseEncryptionServiceTests: injected timeouts that also arm during
setup.** #1483 diagnosed and fixed exactly this in the quarantine-restore
test, but two sibling tests kept the shape. Their injected
ordinaryResponderHandshakeTimeout (0.04 and 0.06) also arms during the
establishSessions setup handshake, where bob is the responder — so a
preempted runner fires it mid-setup, tears down the half-open responder,
and message 3 gets answered as a fresh initiation. The CI failure named
setup's own `#expect(finalMessage == nil)` seeing a 96-byte message 2,
which is precisely the signature #1483 recorded.

Raised both to 1.0s, matching #1483's remedy: the scenarios still need the
responder timeout to fire, and it still does, just with room for setup to
complete first. Thin 1-second waitUntil budgets in the file now use the
shared TestConstants.longTimeout; every one of them backs a positive
assertion, so waitUntil still returns the moment the condition holds and
nothing gets slower in the passing case.

No assertion weakened in either file. Verified 3x sequentially and 3x with
all 18 cores saturated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Raise two more starvation-prone test deadlines

Both surfaced on the CI runs for this PR and #1487, both in tests neither
PR touches, both the same shape as the two already fixed here: a deadline
sized for the work rather than for a runner executing many suites at once.

VoiceNotePlaybackControllerTests waited 5s for a @MainActor Task that
playback schedules for the session acquire and its failure path. When that
Task is not scheduled in time the helper reports *two* failures — the wait,
and the `!isPlaying` the un-run failure path has not reset yet — which
reads like a playback bug rather than a starved scheduler. That is exactly
what CI showed.

GeoRelayDirectoryTests waited 10s for work the directory runs in
Task.detached(priority: .utility); utility priority competes with every
other suite. The retry-scheduling case timed out at exactly 10.06s with the
retry never scheduled, reading like a missing retry rather than a starved
background task.

Raised to 30s each with the reasoning recorded at the helper. Both helpers
return as soon as their condition holds, so nothing slows down when tests
pass — only the genuine-failure case takes longer to report.

Verified 3x with all cores saturated: both suites clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Make the flake class unrepeatable, not just fixed

Raising four deadlines fixed the four tests that happened to fire. The
class was still there: fourteen separate waitUntil helpers, most defaulting
to 1.0s, plus wait call sites with literal budgets. The fifth instance
would have landed the same way, on someone else's unrelated PR.

The rule, now written down in TestConstants.settleTimeout: **a wait
deadline is not a latency budget.** It exists so a genuine hang eventually
fails the suite, so size it for the worst-case scheduler, never for how
long the operation should take. Waits return as soon as their condition
holds, so a generous deadline is free in the passing case and only extends
genuine failures.

- Every wait helper now defaults to TestConstants.settleTimeout (30s), and
  the literal wait call sites below the floor were converted too — sixteen
  sites across ten files.
- TestTimingHygieneTests enforces it by scanning the test sources: wait
  defaults and wait call sites must be at least minimumSettleTimeout, and
  no test may assert an upper bound on elapsed wall-clock time (the
  assertion that started this, which cannot separate correct behaviour from
  a slow machine).
- Both rules waive per line with "test-timing-ok: <reason>", accepted on
  the line or in the comment block above it so the reason has room to be a
  sentence. One legitimate use so far: a NEGATIVE wait in NoiseCoverageTests
  asserting a promotion has *not* completed within 50ms, where a long
  deadline would only make the suite slow while still passing.

Injected production timeouts are deliberately not matched — the Noise
handshake timeouts are the behaviour under test, and short values are
correct there.

Verified the guard actually fails: a canary file with both banned shapes
was flagged with file and line, and the suite went green again once it was
removed. Full suite passes with all 18 cores saturated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Close the guard's blind spot: named short timeouts

A fifth flake landed on CI while the first guard was in review —
GossipSyncBoardTests timing out at 1.03s — and the guard did not catch it,
because the deadline was `TestConstants.shortTimeout` rather than a
literal. A literals-only scan cannot see a short value behind a symbol,
which is the more common way it is written: 81 wait sites used
shortTimeout (1s) or defaultTimeout (5s), every one of them below the
floor.

Rather than guess which of those were safe to raise, all 81 were converted
and the suite timed. Runtime went 15s -> 72s, which located the genuine
negative waits precisely: ten tests that assert something does *not*
happen and therefore always run their deadline out. Measurement instead of
a heuristic, since a mis-guess in either direction is invisible — too
short reintroduces the flake, too long silently costs a minute a run.

Those 28 sites now use `TestConstants.negativeWaitWindow`, a named
constant whose doc explains the inverted reasoning: for a negative wait,
starvation can only make the assertion *more* likely to hold, so short is
correct, and the name states the polarity instead of leaving a bare
literal that reads like the mistake. Suite is back to 15.3s.

The guard now also rejects `timeout: TestConstants.shortTimeout` and
`defaultTimeout`, while accepting `negativeWaitWindow` by name.

Re-verified with a canary carrying every banned shape — literal default,
both named constants, and an elapsed-time upper bound. All four were
flagged with file and line; the suite went green again on removal.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Delete shortTimeout now that nothing may use it

The hygiene guard bans `TestConstants.shortTimeout` at every wait site
and the last users were converted, so Periphery correctly flagged the
constant itself as dead and failed CI. Remove it from both TestConstants
copies; the banned-name entry stays so the symbol cannot quietly return.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-07-26 23:45:20 +01:00
jack
2360140760
Transitive verification: vouch for verified peers over Noise (#1380)
* Add capability bits to announce TLV

Announces now carry an optional capabilities TLV (0x05): a little-endian
bitfield with named bits for upcoming features (prekeys, wifiBulk,
gateway, groups, board, vouch, meshDiagnostics). Old clients skip the
unknown TLV; peers without it decode as nil so features can distinguish
"legacy peer" from "advertises nothing".

PeerCapabilities lives in BitFoundation with a minimal-length encoding
that preserves unknown bits for forward compatibility. Peer capabilities
are stored in the BLE peer registry on verified announce and exposed via
BLEService.peerCapabilities(_:). The local advertisement set is empty
until each feature ships its bit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Transitive verification: vouch for verified peers over Noise

When a Noise session establishes with a peer I verified and that peer
advertises the .vouch capability, send signed attestations (up to 16,
most recently verified first, at most once per peer per 24h) for the
OTHER fingerprints I verified. Receivers accept vouches only from
senders they verified themselves, verify the Ed25519 signature against
the sender's announce-bound signing key, and surface the result as a
new derived trust tier: vouched (unfilled seal) between casual and
trusted.

Protocol:
- NoisePayloadType.vouch = 0x12 carries a batch of TLV attestations:
  voucheeFingerprint (32B), voucheeSigningKey (32B), timestamp
  (uint64 ms BE), Ed25519 signature over
  "bitchat-vouch-v1" | fingerprint | signingKey | timestamp.
  The voucher is implicit in the authenticated session.
- PeerCapabilities.localSupported now advertises .vouch.

Storage (SecureIdentityStateManager / IdentityCache):
- vouches keyed by vouchee, capped at 8 vouchers each; validity is
  recomputed on read (voucher still verified-by-me, < 30 days old), so
  unverifying a voucher retires their vouches without cascade deletes.
- New IdentityCache fields are Optional so pre-existing encrypted
  caches decode cleanly; TrustLevel.vouched is inserted mid-ladder but
  raw values are strings, so persisted values are unaffected (and
  vouched itself is never persisted).
- Panic wipe clears vouch state with the rest of the identity cache.

UI: unfilled checkmark.seal badge in the mesh peer list (filled seal
stays exclusive to verified) and a "vouched for by N people you
verified" section with voucher names in FingerprintView; VoiceOver
labels and xcstrings entries included.

Tests: attestation encode/decode + signature (forged/tampered/expired),
accept-policy gates, batch cap, trust-level derivation incl. voucher
invalidation, persistence compat, and coordinator exchange/accept
policies. Full macOS suite: 1088 tests passing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Fix CI deadlock in vouch tests and live-refresh the fingerprint sheet on vouch acceptance

Two fixes for PR #1380 review findings:

1. CI "Run Swift Tests (app)" hang (exit 137): the new
   SecureIdentityStateManagerVouchTests suite was nonisolated, so Swift
   Testing ran its tests in parallel on the Swift Concurrency cooperative
   pool. Each test enqueues a queue.async(.barrier) write (setVerified)
   and immediately blocks in queue.sync / queue.sync(.barrier)
   (recordVouch / effectiveTrustLevel). On CI's few-core runners every
   cooperative-pool thread ended up parked behind a pending barrier that
   never got a dispatch worker, deadlocking the whole test process until
   the watchdog SIGKILLed it. The suite is now @MainActor, matching the
   production isolation of the vouch API (ChatVouchCoordinator is
   @MainActor) and keeping blocking syncs off the cooperative pool.

2. Codex P2: an open fingerprint sheet did not refresh its vouched badge
   when a vouch batch was accepted - VerificationModel.bind() never
   observed the trust-change signal. It now subscribes to the
   "peerStatusUpdated" notification that
   ChatVouchCoordinator.notifyPeerTrustChanged() posts (same source
   PeerListModel uses) and forwards it to objectWillChange. Added a
   regression test that pins VerificationModel's own subscription
   (verified to fail without the fix).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Skip media-wipe detached tasks under tests (shared-filesystem race)

panicClearAllData and clearCurrentPublicTimeline delete the real
~/Library/Application Support/files tree in detached utility-priority
tasks. The SPM test process shares that tree and ChatViewModelTests
invoke both methods, so under parallel scheduling the wipe lands at a
nondeterministic time — deleting media a concurrently running test just
wrote (and the developer's real app data with it). Guard both with the
existing TestEnvironment.isRunningTests pattern, mirroring the same fix
on feat/mesh-diagnostics (#1377).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Port vouch capability-race fix to feat/vouching (ports b8adcbe9)

Ports the on-device-confirmed fix from the integration test branch
(commit b8adcbe9) onto feat/vouching so PR #1380 is actually correct.
On-device testing confirmed the transitive vouch propagated once the
send was triggered on verify / announce arrival rather than auth alone.

Vouch attestations only ever sent from peerAuthenticated, gated on the
peer's .vouch capability. That capability arrives via the peer's announce,
processed independently of the Noise handshake, so at auth time the set was
usually empty -> gate failed -> vouch silently skipped and never retried.

- Refactor the send path into a reusable attemptVouch(to:fingerprint:now:).
- Trigger on peer-list updates (peersUpdated): fired after every verified
  announce, so the batch goes out once the .vouch bit actually arrives.
- Trigger on local verification (vouchToConnectedVerifiedPeers): verifying a
  peer runs a vouch pass over connected verified peers, covering the
  verify-while-connected case and propagating the new identity onward.
- Relax the capability gate: treat an empty/unknown set as eligible (the
  Noise 0x12 payload is ignored by non-supporting peers); only skip when a
  non-empty set explicitly lacks .vouch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-07 14:48:37 +02:00