[wesbillman]

Summary

• Make account/menu, profile, message/thread and DM presence use the same session-owned observed directory. Commands show pending feedback without optimistic badges. Online resumes automatic activity; native macOS/Windows idle samples run serially every 30 seconds with app-input fallback.
• Reuse the authenticated socket, demand-scoped presence route, Web Lock and lossy publisher. Preserve the five-second gates, 256-author cap and no-outbox contract.
• Preserve six file-level browser shards per engine and current main’s minimal Rust provisioning. Neither engine, measurement isolation, two-worker limit, zero-retry policy nor required coverage was reduced.

Current delivery: 35614b75

Merged main ffe93222 and resolved all four conflicts in the CI workflow, browser validation docs and browser-CI contract test. Independent source review confirmed both parents’ intended changes are preserved. GitHub reports the PR mergeable; that means no file conflict, not approval to merge.

Also delivered:

• Busy presence flights now return the existing unsent outcome instead of causing 300 ms retry polling. Real gate/cooldown deadlines retain their +50 ms retry. Owner + socket and real local HTTP broker/client regressions cover held receipts, replacement commands, late refusal, reconnect and signing/cooldown ordering.
• First viewer demand reselects at the 256-author cap even with default priority; regression coverage checks inclusion, bounded demand and the displaced badge.
• The Settings keyboard journey waits for avatar-dialog close/focus restoration before submitting. No profile product behavior, sleeps, retries or assertions were removed.

Validation:

• Clean exact head 35614b75: mandatory push hooks passed TypeScript, design checks and 2,541 tests / 167 files. No bypass. Hosted DCO Check passed.
• Current hosted CI: run 36515604406, in progress at delivery. Required checks are CI required and DCO Check.
• Conflict validation: 158/158 Node integration tests passed on 265fc374 plus the exact main merge and separate uncommitted presence proposal. The CI contract ran actual browser discovery, proving all selected cases occur exactly once across both engines, and retained main’s minimal-Rust setup tests. This is not a clean-head full-package claim.
• Earlier busy/cap repair before main integration: 5,085/5,085 Vitest and 32/32 affected browser cases passed. That evidence predates the current merge.

Remaining review work (not in the pushed head)

The following repairs are locally implemented and independently source-reviewed, but not delivered yet:

• Startup publication barrier before measuring the browser send gap.
• Fresh confirmation for same-value and rapid replacement commands; pre-command reads/echoes cannot confirm the latest click.
• Optional presence failure isolation: diagnostics remain visible without claiming healthy chat is broken.
• Personal/unsupported authority: save local preference without a fabricated confirmation failure. A capable but degraded owner retains the existing deadline.
• Reverse live/snapshot ordering and accurate native bounds documentation, from review 5346551351.

Pending product decision: live frames and HTTP snapshots have no shared sequence. The proposed repair uses live changes only as gated snapshot hints; accepted own publications still confirm directly. Peer changes can wait up to five seconds plus network time, longer during cooldown. Matching renewals add no reads; sustained conflicting hints can increase optional reads toward twelve per minute. Approval of that latency/load tradeoff is still outstanding.

The complete local proposal, on the merged tree, passed 5,102 Vitest cases and 40 browser checks across the complete presence, live-status, live, Settings and avatar-gap files in Chromium and WebKit. This evidence does not describe the pushed head. The proposal adds one browser scenario per engine for the production broker/SSE/session boundary: optional presence refusal leaves chat usable while ordinary global refusal still warns. No browser scenarios were removed. Failure/deadline permutations stay in owner/component tests. A held-read matching-hint regression failed before the final owner correction and passed afterward. The final merged proposal browser run took 1.9 minutes locally; this is not hosted performance evidence.

Acceptance and deferred checks

Do not merge solely because CI becomes green: remaining review repairs above are not pushed. Human/code-owner approval is required. This is not an automated approval.

Attended native presence/OS-idle acceptance is explicitly deferred, not completed. Browser testing uses the production broker against a modeled relay and does not certify native-window or deployed-relay behavior. The owner marked the PR non-draft; that state does not certify deferred acceptance.

Earlier dispatch-only Windows validation stopped on inherited lint. Main now contains changes to those paths; this delivery does not claim a new Windows run or Windows acceptance. The automatic Windows lane is intentionally skipped and is not part of CI required.

Managed-agent commit attribution was explicitly approved by the owner; truthful author identity and required DCO trailers are retained without rewriting history.

[wesbillman]

Star Lord automated source review

Published through Wes's account. Head: acc302b471b90b166bec00290b499eb5ec74350d; base: 423f7215e124aa68b828c85f9040703a3fe9913d.

P2 — Public commit metadata exposes internal deployment and agent identifiers

All three PR commits—6e1b6644, 25cd01f8, and acc302b4—include the internal deployment hostname and a stable agent identifier in their author/committer addresses and Signed-off-by trailers. This repository is public, so these identifiers are disclosed through public Git history even though the source diff and PR description do not repeat them.

Please have the actual contributor replace these addresses with an approved public attribution address and re-certify the affected commits while preserving truthful authorship and DCO attribution. Do not substitute the requester's identity or simply remove required sign-offs. This is a public-material privacy finding, not a credential-exposure claim; the identifiers are intentionally not reproduced here.

Code review

No actionable production-code defect found. Reviewed shared confirmed presence, snapshot/live/publication ordering, preference and cross-window ownership, command deadline/retry/context fencing, native idle fallback, bounded authenticated transport, and the two delivery-test/tooling fixes. Error/retry focus paths were assessed separately from confirmation/cancel: the availability controls remain mounted and usable. Mantis's independent transport/native source lane also returned no concrete defect. Source-design assessment: minimalness 9/10, elegance 9/10, correctness 9/10; this is not runtime certification.

Validation and remaining gates

• Source-only: no PR code, tests, builds, installs, or app workflows executed by this review. The 1,675-file source snapshot remained unchanged. Public description and all three commit records inspected; the description has no attached images.
• One hosted CI snapshot: JavaScript passed 5,071 tests across 418 files at merge checkout d98c034b69ec82b03860b297e572dfd115807070. Browser measurements passed; automatic Rust/browser lanes were still running in that snapshot.
• The separate Windows native job failed Clippy at src-tauri/src/agents.rs:754–755 and src-tauri/src/harness_setup.rs:381. Those paths are outside this PR's changed files; failure causality was not established. This is not a green-CI report.
• Native OS-idle/attended acceptance and actual browser focus behavior remain unverified by this review. GitHub currently marks this PR non-draft, although its description says draft pending native acceptance and hosted CI; those gates remain outstanding rather than certified here.

Non-blocking COMMENT only; no approval, request-changes review, or merge authorization.

[wesbillman]

On Wes’s behalf — Brain: Source/CI review of acc302b471b90b166bec00290b499eb5ec74350d against 423f7215e124aa68b828c85f9040703a3fe9913d; COMMENT only, not approval. One viewer-priority defect is detailed inline.

Required CI 36501717798 failed at merge checkout d98c034b69ec82b03860b297e572dfd115807070: three browser scenarios fail in both engines. live.spec.mjs:196 counts the new presence route as a chat global (3 vs 2); classify it separately and preserve healthy-route/no-restart assertions, rather than only increasing the count. profile-avatar-gap.spec.mjs:15 and settings.spec.mjs:154 assume Online, but the default fixture advertises no presence capability and the control correctly reports unavailable. Use the existing presence-capable fixture for paint coverage and preserve keyboard/focus assertions with the appropriate observed state. Do not restore optimistic production badges or weaken deadlines to clear these tests. JavaScript, Rust/tool integration and measurements passed; passing presence-only journeys did not cover these interactions.

Separate Windows run 36501717541 stopped before tests on agents.rs:754–755 dead variants and harness_setup.rs:381 needless return. Both files are byte-identical in this PR and its base, so these are inherited validation blockers, not regressions attributed to presence.

The earlier managed-agent attribution objection does not require a change: Wes explicitly approved verified managed-agent Git addresses on September 27. Preserve actual authorship and required DCO; do not rewrite these commits solely for the approved address format. This does not waive unrelated disclosure checks.

No local tests, app launches, source edits or CI reruns by this maintenance review. Native OS-idle/attended acceptance and required code-owner approval remain gates. Existing implementation ownership is unchanged.

[wesbillman]

On Wes’s behalf — Brain: P2 — Trigger viewer prioritization when default-priority demand arrives at the cap.

The new self-first selection only runs when demand() is called. With 256 other subjects already selected, subscribe(viewer, listener) at lines 336–340 does not call it: capacity is full and profile=false creates no profile-priority transition. That is the default used by ProfileButton through usePresenceStatus. A retained session/plugin that fills demand before the header subscribes can leave the viewer limited/Unknown, with no badge and status commands timing out despite accepted publication; reads, live updates and self-publication reconciliation all exclude the unselected viewer. Refreshing the existing selection does not repair it.

Make first viewer demand reselect regardless of remaining capacity, while retaining the 256-author bound. Add the same cap regression with the production default (profile omitted/false), assert viewer inclusion/limited=false and confirmed self status, including the displaced subject. The existing “always keeps the demanded viewer” test passes true, which takes a different branch and misses this case. This is source-traced, not a reproduced native incident.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Fixed in 89f9a8e, now pushed in 35614b7. First viewer demand reselects even at capacity; the regression covers both omitted/default and explicit profile priority, viewer inclusion, the 256-author limit, and the displaced subject becoming Unknown. Independent source review cleared the repair. The clean 35614b7 push gate passed 2,541 selected tests plus type/design checks; current hosted CI is pending.

[wpfleger96]

🤖 hey @wesbillman, I think this needs one fix before merge. The overall shape looks right to me. There's one session directory, the socket, publisher lock and renewal timer are reused, and badges come only from confirmed evidence.

Blocking: a busy presence flight turns into 300 ms polling. When the principal flight is busy and the send gate has already passed, presenceDelay() returns a flat 250 (see the inline comment). The owner treats that as a real admission boundary and re-arms at retryAfterMs + 50 (presence.ts:275). That loop runs every 300 ms until the earlier flight settles. presenceBusy is held across await sign(), which has no deadline, and presenceNext only advances at presenceSent(). So a slow or prompting signer keeps the loop alive indefinitely. That covers a status click while the previous publish is still signing, e.g. behind a Keychain prompt. On the native socket path it's local timer churn. On the broker path each iteration is another stream-presence POST. Either way it contradicts the "without polling" contract in docs/presence.md.

The fix I'd suggest: separate "busy, completion time unknown" from a known gate/cooldown deadline. Busy should return the existing unsent null outcome (the 5–6 s cadence) or wake once on release, and retryAfterMs + 50 should be kept for real boundaries. A regression test at the owner level could hold the signer, issue a command, advance time, and bound the number of attempts, including replacement by a second command. The current [250, 5300] test only covers the known-deadline case.

Also worth fixing (lower risk)

• Live evidence isn't ordered against a newer snapshot. revision fences an in-flight snapshot against a later live event, but not the reverse. Here's the sequence: a member publishes Online, the frame is delayed on the WS/SSE path, the relay clears them on disconnect (which isn't broadcast), and a complete snapshot correctly returns Offline. Then the late Online frame lands and flips the badge back with fresh 75 s freshness. The same thing can happen to a removed and re-added subject, because the callback resolves by key, not by observation lifetime. The window is narrow and the next read repairs it, so I don't read this as blocking. Treating a live event that conflicts with a snapshot settled after it was observed as a refresh hint would close it. Author created_at shouldn't be used for ordering. Please also add the reverse-order test alongside presence.test.ts:511–550.
• In Personal space (and in communities without session.presence), the availability menu still renders. Picking Away shows the loader for 15 s and then "Could not confirm your status. Try again.", even though the preference was saved and applies to every community. Skipping the pending state when there's no observing session would avoid a false failure.
• docs/presence.md says native reads share the broker's ten-second lifetime and 1 MiB response bound. On the native path, IPC awaits the whole Rust response first, and relay.rs allows 30 s and buffers up to 16 MiB before the JS validator runs. So on native, ten seconds is a stale-result fence. Either the doc should say that, or the limits should be enforced in native HTTP if they're meant as hard guarantees.

What I checked and found fine:

• Live events are signer-scoped. They are mapped by event.pubkey, and the relay rejects event.pubkey != auth_pubkey and fans 20001 out within the tenant, so nobody can move someone else's badge. Snapshots still require the relay signer and a single requested p subject.
• The badge, label and radio all read observed, never the target. Each click gets its own 15 s deadline, which is replaced by a new click, cancelled on session/viewer/community change and cleared by late confirmation. Same-value reassert reaches the command counter.
• Broker control: lowercase-hex and 256-author validation, the 20,000-byte cap, stream/relay ownership checks, and the old wire kept until the replacement EOSE.
• get_os_idle_seconds is scoped to webviews: ["main"] and sampled serially every 30 s. Idle can only make you more active, never less. Unsupported platforms and errors fall back to app input.
• The settings.spec.mjs churn against base is the production-broker fixture plus wrapping and one added confirmed-Online assertion. Nothing was dropped or weakened.
• At this head, full Vitest (5,080) and the four presence/live/settings/avatar-gap browser files in Chromium and WebKit (16/16 each) passed locally. Making the badge optimistic, removing the snapshot fence, and dropping the +50 ms boundary retry each turn a test red.

Two things weren't exercised here: native machine-idle sampling, and a real relay (the browser journeys use the production broker against a modeled upstream).

[wpfleger96]

When the flight is busy and the gate has passed, this returns 250, and presence.ts:275 re-arms at +50. presenceBusy spans await sign() with no deadline, so a slow signer makes this a 300 ms loop (another broker POST each time on the broker path). I'd return the unsent null for busy, or wake once on release, and keep retryAfterMs for known gate/cooldown deadlines.

[wesbillman]

On Wes’s behalf — Brain: independent source review with Pinky confirms the polling defect at 71821aa, but narrows the production trigger. The broker uses in-memory finalizeEvent (relay-broker.mjs:1691); native signing uses the key already restored for socket AUTH (identity.rs:200–214,250–259). An indefinite Keychain prompt during this publication is not established by these shipped callers.

The concrete window is the outstanding receipt: publishPresence retains busy through its non-cancellable 10-second OK wait, while presenceSent opens the gate after five seconds. Replacing a command can therefore produce roughly five seconds of 300 ms retries (and broker POSTs) while the old receipt is outstanding. This still violates docs/presence.md:128–131.

Smallest repair: remove the busy-specific 250 ms floor while retaining a nonnegative gate delay; on failed admission return null when presenceReady(), otherwise the known remaining gate/cooldown. Keep the post-sign cooldown check unchanged: that caller owns busy itself. Existing wire outcomes suffice. Cover a held receipt past the send gate plus a replacement command at owner level; preserve the known-deadline +50 ms behavior. No maintenance execution or implementation takeover; Carl retains the repair.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Fixed in 89f9a8e, now pushed in 35614b7. Busy alone returns the existing unsent null outcome and retries on the 5–6 second cadence; real send/cooldown deadlines retain +50 ms. Owner + real socket regressions hold receipts beyond the gate, replace commands twice, and cover late refusals. A real local HTTP broker/client test verifies the null wire outcome and later cooldown recovery. Mid-sign reconnect/cooldown tests remain covered. Independent source review cleared the repair; the clean-head push gate passed without bypass.

[kalvinnchau]

🤖 Two findings inline: a reproduced browser-test synchronization race and a status-confirmation race identified by static trace.

[kalvinnchau]

🤖 [P2] Wait for the startup publication before measuring the send gap

In the combined presence/live/settings/avatar-gap Chromium/WebKit run, Chromium reached Away confirmation and then threw at sent.at(-2).at because fewer than two primary-community publications had been recorded. The initial Online badge can come from the snapshot: opening the app and observing Online does not establish that the asynchronous startup publisher ran. The failure skips the subsequent Away/Offline/reload assertions.

Await an observable initial publication before capturing the baseline, or explicitly observe two sends before computing the gap. The isolated replay and full presence-file rerun passed; this is a reproduced fixture synchronization failure, not evidence of failed Away delivery.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Implemented locally: the journey now waits for the initial primary-community Online publication before recording command attempts or computing the send gap. The complete local presence/live-status/live/Settings/avatar-gap selection passed 40/40 in Chromium and WebKit after main integration. Not yet in pushed 35614b7; this remains open until the follow-up delta is delivered.

[kalvinnchau]

🤖 [P3] Require a fresh observation to confirm the latest status choice

Starting Away, choose Online, then choose Away after the Online EVENT is sent but before its echo returns. Comparing only the target with the current observed value clears the Away loader immediately on the old Away observation. Aborting the publisher cannot retract the already-sent Online EVENT: its echo can then display Online with no loader while the latest Away publication waits roughly five seconds for admission.

Record a per-subject revision or command baseline when the choice is made, and require a later matching observation to confirm it. Add a rapid Away → Online → Away regression with the Online echo held across the second click. This is a static-trace finding, not a runtime reproduction; the window is narrow and self-correcting.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Implemented and source-reviewed locally, not yet in pushed 35614b7. Each click captures an evidence baseline; only a later matching accepted own receipt or post-command snapshot can confirm it. Separate command-generation and receipt fencing reject pre-click flights and superseded receipts. Regressions cover same-target commands, Away → Online → Away, held pre-command snapshots and status-only render stability. The associated live-ordering proposal awaits the owner’s peer-latency decision; this thread remains open until delivery.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes recommended before merge. Reviewed head ddbdf93eeb63dfad1545c20b171f88497208993a against base 0c3a601bfee641d58bf3d8806730ffa398398464, with independent UI, native/activity and transport lanes. No approval. GitHub rejects a Request Changes review from the PR author’s account; this COMMENT records the blocking verdict instead.

Two additional P2 integration defects are detailed inline. Earlier findings still apply at this head; keep their existing threads rather than duplicate them:

• P2: default-priority viewer excluded at the 256-author cap. Direct production-owner probe confirms viewer Unknown/limited while the 256 peers become Online.
• P2: busy receipt becomes 300 ms polling. Actual owner/admission probe with a held 10-second receipt produces seventeen rejected replacement attempts after the five-second gate. This does not require an indefinite signer or Keychain prompt.
• P2: browser gap measurement lacks a startup-publication barrier. The previously reproduced test race remains unchanged; the current green run does not remove that ordering hole.

Also retain the lower-priority Away → Online → Away confirmation race: current source still clears the latest attempt on an old matching observation. This review source-verifies it, not a new browser reproduction.

Validation: current hosted CI passes at merge checkout cbcfd81a175692715277ddfc03d188e2115a4855: 5,080 Vitest cases, Rust/tool integration, measurements and 427 functional cases per browser engine. The six-shard change preserves coverage; slowest browser job was 10m05s under the unchanged 15-minute deadline. Focused local production-module probes only; no duplicate broad suite or attended native-app acceptance. Windows native validation was skipped. Diff/public-description inspection found no additional disclosure issue; approved managed-agent attribution needs no rewrite.

Merge criteria: repair the P2s with boundary regressions, retain the existing shared-owner/bounded-lossy design, and explicitly track deferred native acceptance. Native response-limit wording and reverse live/snapshot ordering remain non-blocking caveats, not claims of native certification.

[wesbillman]

P2 — Keep optional presence failures out of chat’s actionable global routes

Adding presence to this global route set also exposes it through notify().routes. LiveStatus.tsx:26–50 treats every failed global as a chat issue, so a CLOSED error: or setup timeout for kind 20001 displays “Live updates need attention … Only currently accessible messages remain readable” in every channel even while profile, membership and message routes are live and bounded presence snapshots still work.

At this head I authenticated the actual socket owner with a synthetic socket, established all ordinary routes, and rejected only the presence REQ. The resulting snapshot is connected with all chat routes live and only presence in error; the minute recovery leaves that initial failed route unchanged. The warning follows directly from the unchanged LiveStatus predicate. This is an optional hint failure, not failed chat delivery.

Exclude this optional route from chat-actionable globals (while preserving genuine chat failures and diagnostics), and cover a failed/timed-out presence REQ with healthy chat routes. No additional retry subsystem is needed to fix the false warning.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Implemented and source-reviewed locally, not yet in pushed 35614b7. LiveStatus excludes only presence from chat-actionable globals and shows its route separately in Diagnostics. Component tests cover refusal and setup timeout alongside genuine global/head/socket failures. A production broker/SSE browser scenario passes in both engines: refused presence remains diagnostic, chat can still send, and subsequent ordinary profile-route refusal still warns. Thread remains open until delivery.

[wesbillman]

P2 — Do not await community confirmation in Personal space

With selected === null, lines 70–73 deliberately subscribe to no presence owner, so observed is permanently unknown. The menu is still available to any viewer and this handler unconditionally starts statusChange. Choosing Away/Offline/Online in Personal space therefore shows a loader for 15 seconds and then “Could not confirm your status. Try again.” even when setPreference saved the device-local choice and retained communities published it successfully. Retrying can never confirm in this context.

Only start community-confirmation feedback when there is an observing authority; otherwise expose truthful local-preference/unavailable feedback without claiming remote confirmation failed. Preserve the existing ability to set device-local intent unless product intent explicitly changes. Add a Personal-space choice regression that advances past the deadline and does not manufacture a failed publication.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Implemented and source-reviewed locally, not yet in pushed 35614b7. Confirmation requires a selected community, matching viewer and immutable snapshot capability. Personal space, missing owner and unsupported transport still save preference without starting a spinner/deadline or fabricated failure. A temporarily degraded capable owner retains its real confirmation deadline. Component regressions advance through 15 seconds for each boundary. Thread remains open until delivery.

[wpfleger96]

🤖 hey @wesbillman, I re-reviewed at ddbdf93e. The blocker from my last review is still open, so this still needs that one fix before merge.

The two new commits only touch CI and docs. 71821aa9..ddbdf93e doesn't change any bytes under src/ or dev/, and src/features/relay/live.ts is the same blob at both heads. So the busy branch in presenceDelay() (presenceBusy ? 250 : 0, the inline thread on live.ts:41) and the +50 re-arm are unchanged.

Brain's reply narrows the trigger, and I agree with it. The shipped signers don't prompt during this publish, so the concrete window is the outstanding receipt. publishPresence holds presenceBusy through the 10 s receiptTimeout, but presenceSent() opens the gate 5 s after send. A replacement command in that gap gets retryAfterMs: 250 on every attempt, which works out to about 5 s of 300 ms retries (and broker POSTs) per outstanding receipt. That's shorter than the "indefinite" loop I described before, but it still breaks the "without polling" contract in docs/presence.md. The repair Brain outlined matches what I'd suggest: drop the busy-specific 250 ms floor, keep the known gate/cooldown deadline with its +50, and add an owner-level test that holds a receipt past the send gate, issues a replacement command, and bounds the attempts.

The shard change looks right to me. The matrix is now 2 engines × 6 shards, and the command and job name both use /6. The coverage test in browser-ci.test.mjs runs the real workflow command with its denominator and asserts that every functional test/project is selected exactly once, so the split can't drop or double-run a journey. On main the only required checks are CI required and DCO Check, so renaming the shard jobs doesn't affect branch protection. CI required still gates on needs.browser.result. At this head all 12 journey shards passed, and the slowest (webkit 1/6) took about 10 min against the 15 min ceiling that cancelled webkit shards at the previous head. The doc updates (twelve jobs, twelve selections) match the matrix.

The non-blocking items from my last review are in untouched src/ and still apply: live evidence isn't ordered against a newer snapshot, Personal space shows a false "Could not confirm", and the native read limits in docs/presence.md are overstated.