Skip to content

perf(desktop): publish mention sends before waking agents - #7154

Open
matt2e wants to merge 16 commits into
mainfrom
faster-agent-sends-option-2
Open

perf(desktop): publish mention sends before waking agents#7154
matt2e wants to merge 16 commits into
mainfrom
faster-agent-sends-option-2

Conversation

@matt2e

@matt2e matt2e commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Cuts perceived agent-mention send latency by publishing the message first and waking the agent afterwards, instead of blocking the send on a synchronous agent start/deploy round-trip. A send that mentions a stopped or undeployed managed agent now shows the message immediately; the wake runs fire-and-forget after the relay accepts the publish. The already-running-agent send also gets faster via revalidation dedupe and NIP-11 caching.

Changes

Publish-first agent wake

  • Wakes for mentioned managed agents are collected during send preparation and flushed fire-and-forget only after await send(...) resolves. No start can fire — and no "your message was sent" toast can appear — for a message the relay never accepted; every abort path (cancel, readiness error, publish rejection, dismissed non-member prompt) simply drops the queue. Persona-create wakes ride the pending draft behind the non-member prompt for the same reason.
  • Each wake is bound to the tenant scope captured at send time: the new useDetachedAgentStart hook passes expectedRelayUrl + expectedSignerPubkey with every start, so a wake that outlives a community switch fails closed at the backend instead of spawning against the new tenant. A wake whose scope has not resolved yet (identity query still loading, blank stored relay URL) is refused with a recoverable toast rather than fired unscoped.
  • In-flight wakes are deduped through a module-level map keyed by (relay URL, pubkey) — the same tenant pair the backend keys on — so two quick sends or two composers cannot double-spawn a cold agent during the seconds-long start window. Entries are deliberately retained across community switches (the key is the tenant scope, so a retained entry can never affect another community, and clearing it let an A→B→A round trip deploy a provider agent twice) and self-clean when the start settles.
  • Wake-failure toasts are fenced to the community they fired in via a module-level scope mirror: a start that settles after a community switch logs instead of rendering community A's failure over community B's UI, and an A→B→A return re-delivers the warning where it is actionable.
  • Membership attach and access-policy writes stay synchronous, so the harness's first kind-39002 read still sees the channel.

Replay floor

  • The send timestamp travels with the wake as BUZZ_ACP_REPLAY_FLOOR, threaded through both local spawns (spawn_agent_child) and provider deploys (deploy_to_provider injects it into launch.policy_env), so the harness's startup watermark replays back past the just-published triggering message no matter how long the spawn takes. buzz-acp clamps the floor to [now − 15min, now].
  • The floor is captured at enqueue time, not flush time — the flush runs post-publish, so a flush-time stamp could exceed the message's created_at and skip the very message the floor exists to cover.
  • On local spawns the caller's floor is asserted after the user env layering (and the ambient parent-process value is stripped unconditionally), so a saved persona/global/agent env entry cannot shadow this send's floor — mirroring the shadow-strip the provider path applies to launch.env. Both halves share one REPLAY_FLOOR_ENV_VAR const.

Send-path latency reductions (already-running agents)

  • Mention revalidation is deduped: the publish-boundary pass reuses the pre-side-effect authorization pass unless an awaited round-trip actually separated the two (background upload, link-preview settlement, DM expansion, a real access-policy/membership write, or active-huddle enrollment). This preserves the fix(desktop): enforce agent mention authorization at send boundaries #5681 authorization boundary while making the common send single-pass.
  • NIP-11 self lookups are cached per relay URL for 5 minutes. Only verified values are cached — non-2xx and malformed responses stay retryable — and URL keying keeps community switches from serving another relay's identity.
  • applyReusableAgentAccessPolicy now reports its relay write explicitly ({ agent, wrote }) instead of signalling through object identity, so the revalidation trigger above is load-bearing by construction.

File splits

Four files crossed the repository file-size ratchet during this work; one cohesive unit was extracted from each rather than raising a ceiling — runtime/setup_payload.rs, commands/agents_create_fields.rs, app_state_accessors.rs, and useEnsureAgentMentionsReady.ts. The ratchet is green at the tip.

Review follow-ups

The three concrete findings from the first review round are fixed at the tip: the pre-publish wake and its false "your message was sent" toast (fixed by queueing wakes behind the publish), the stale cross-community failure toast (fixed by the scope-mirror fence), and the A→B→A duplicate provider deploy (fixed by retaining the tenant-keyed in-flight entries across switches). The fast-path admission-staleness point is answered in the review thread: deferred paths already re-validate at the publish boundary, and the remaining fast-path window is milliseconds against an irreducible network-transit race.

Mid-branch send-perf instrumentation was added to attribute the residual spinner latency and reverted once that analysis concluded — it is net-zero in this diff.

Deferred follow-ups

Durable mention catch-up via event_mentions (option 2 step 3) and backend deploy-epoch coalescing for the wake paths that do not funnel through useDetachedAgentStart (Agents-panel Start, restore, inbound-persona deploys) are intentionally left for separate changes.

Testing

  • cargo test --lib on desktop/src-tauri: 3054 passed; clippy -D warnings + fmt clean
  • Desktop unit tests: 5856 passed (the 5 failures are the pre-existing inboxReopenNavigation / useRetainedProjectGitViews baseline, present on origin/main); tsc --noEmit and biome clean
  • Full mentions (87), channels (89), and community-rail (25) Playwright smoke suites against pnpm build:e2e bundles, with 3× stress reruns of each new spec
  • The load-bearing regression specs were confirmed red on the pre-fix code: publish-failure → zero starts and no false toast, the dedupe hold (1 call vs 2), the fail-closed scope refusal, the rail-switch toast fence, and the A→B→A retention spec (1 deploy vs 2)
  • New unit coverage pins the queue contract (enqueue-time floors, attach-seam queueing), the scope capture and verbatim relay-URL handoff, the dedupe map's keying and settle-then-repermit behavior, the unscoped refusal, the toast-scope mirror, the { agent, wrote } contract, and the replay-floor env layering on both spawn paths

🤖 Generated with Claude Code

matt2e and others added 11 commits August 31, 2026 12:18
Mentioning a stopped or undeployed managed agent used to block the send
on the full agent start/deploy round-trip, so the message publish waited
seconds behind process spawn. Detach the start instead (option 2,
steps 1 and 2): the publish proceeds immediately and the wake runs
fire-and-forget off the critical path.

- useMentionSendFlow: start/deploy for mentioned managed agents now runs
  detached via a startAgentDetached callback; failures surface as a
  post-send toast instead of failing the send. Membership attach and
  access-policy relay writes stay synchronous so the harness's first
  kind-39002 read still sees the channel. A pending background start no
  longer gates the composer's next send.
- attachManagedAgentToChannel / createChannelManagedAgent accept an
  opt-in detachedStart hook; other callers keep the awaited behavior.
- Replay floor: the send captures its timestamp and passes it through
  start_managed_agent -> spawn_agent_child as BUZZ_ACP_REPLAY_FLOOR, so
  the spawned harness's startup watermark replays back past the
  just-published message. buzz-acp clamps the floor to now-15min (and
  ignores future floors) before deriving startup_watermark.
- E2E: the five "starts it before sending" mentions specs now pin the
  new ordering (publish lands while start_managed_agent is held pending
  behind an injected delay) and the replayFloorUnix payload; new spec
  covers the post-send start-failure toast; the channels DM spec now
  pins that a failed start no longer drops the expanded DM.

Step 3 (durable catch-up via event_mentions) and the revalidation
dedupe / NIP-11 caching are intentionally left for follow-ups.

Verified: cargo test -p buzz-acp --lib (835 passed), clippy + fmt on
buzz-acp and desktop/src-tauri, desktop tsc --noEmit, desktop unit
tests (5799 passed), and the mentions + channels Playwright smoke
specs against a pnpm build:e2e bundle.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
… self

For the common already-running-agent send, the ~1s of latency was not the
agent wake but two identical revalidateMentionPubkeys passes (~8 sequential
relay round-trips), each fronted by an uncached NIP-11 HTTP GET. This lands
the independent quick win from the faster-agent-sends analysis on top of
the publish-first change.

- useMentionSendFlow: the publish-boundary pass now reuses the
  pre-side-effect pass's admitted result on the immediate path, and only
  re-validates when a deferred wait (background media upload, link-preview
  settlement) separated the two — preserving the #5681 authorization
  boundary where revocation can actually race the publish. Inputs are
  already normalized/deduped, so the substitution is behaviorally
  identical on the fast path.
- fetch_relay_self_at: NIP-11 `self` lookups are now cached per relay URL
  in AppState for 5 minutes. Only verified Some values are cached; non-2xx
  responses and missing/malformed `self` stay retryable so an outage is
  never pinned for the TTL. Keying by URL keeps community switches from
  ever serving another relay's identity.
- E2E: the two specs pinning revalidate_relay_agents at +2 per send now
  pin +1; a new spec pins the deferred-upload path still revalidating at
  the publish boundary (revocation injected mid-upload strips the p tag,
  +2 calls). Three new Rust tests pin cache hit, non-success no-cache,
  and TTL-expiry refetch.

Verified: cargo test -p buzz-lib --lib (3008 passed), clippy + fmt on
desktop/src-tauri, desktop tsc --noEmit, biome check, desktop unit tests
(5799 passed), and the full mentions Playwright smoke suite (80/81; the
one failure is the pre-existing under-load flake in the publish-first
provider-deploy spec, green in isolation) plus a 3x stress rerun of the
dedupe-affected specs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…e after send-path side effects

Address the two review warnings on the publish-first mention-send stack
(f3df4be4f, f0a9915b5), keeping its design intact.

1) Provider replay-floor gap — fixed via the preferred route: thread the
floor into the deploy payload. start_managed_agent's provider branch now
passes replay_floor_unix through deploy_to_provider, which injects it
into the payload REBUILT after the deploy lock as
launch.policy_env.BUZZ_ACP_REPLAY_FLOOR — the same env var a local spawn
sets — so a cold remote harness's startup watermark replays back past
the already-published triggering mention instead of booting blind to it.
No synchronous fallback was needed: launch.policy_env already transports
per-spawn env (effort level, model, session title) to providers, so the
deploy handoff carries the floor cleanly. Any same-named launch.env key
is stripped when a caller floor is present (that tier later-wins
remotely); the floor is invocation state, never persisted, so redeploys
without one never inherit a stale floor. Other deploy callers (create
flow, access reconciliation, inbound personas) pass None and are
unchanged.

2) Revalidation-at-publish window: the single-pass dedupe now reuses the
pre-side-effect authorization pass only when nothing separated it from
the publish. completeSend tracks whether an awaited relay round-trip
actually ran in between — DM expansion (onPrepareSendChannel),
managed-agent access-policy or membership attach writes (reported by
ensureManagedAgentMentionsReady; a matching policy that returns the same
record does not count), or active-huddle enrollment
(sync_agents_to_active_huddle's matched_active_huddle; with no active
huddle it returns before touching the relay) — and re-validates at the
publish boundary when any did. The common send (member agent, no
expansion, no huddle) stays single-pass.

3) startAgentDetached now depends on startAgentMutation.mutateAsync
(stable) instead of the whole mutation object (fresh each render, repo
gotcha #6), keeping the downstream useCallback chain reference-stable.

Tests: five Rust unit tests pin the payload injection (policy_env ride,
case-insensitive launch.env shadow strip, None passthrough, tolerance
for missing launch/policy_env); the in-channel provider E2E spec now
also pins replayFloorUnix on the detached deploy invoke. New E2E specs
pin publish-boundary revalidation for the attach path (revocation
injected while a delayed membership write holds the publish open strips
the p tag; +2 passes with no update_managed_agent) and for a live huddle
on the channel (+2 passes, +1 huddle sync); the managed relay-agent
DM-expansion spec pins +2; the two fast-path specs keep pinning +1.

Verified: cargo test --lib on desktop/src-tauri (3013 passed), clippy -D
warnings + fmt, desktop tsc --noEmit, biome, desktop unit tests (5799
passed), full mentions Playwright smoke (81/83 — both failures are
pre-existing under-load flakes, green in isolation), a 3x stress rerun
of the new specs, and the five DM-expansion channels specs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…path

The send path had zero logging on either side of the bridge, so "the send
still feels slow after publish-first" could only be answered by guessing
which awaited step dominated the spinner. Add info-level, per-send-click
timing on both halves so one test run attributes the latency.

- New `sendPerfLog.ts`: `createSendPerfTimer` wraps each awaited step and
  emits one `[send-perf] completeSend` summary. `useMentionSendFlow`'s
  `completeSend` — the whole spinner window — now times revalidate1,
  managedAgentsLookup, prepareSendChannel, ensureAgentsReady, huddleSync,
  resolvePreviewTags, revalidate2 and publish, alongside the flags that
  decide which of them run at all: channelType, mention/addressed-agent
  counts, isReply, hasAttachments, hasLinkPreviews, wroteRelayState,
  matchedActiveHuddle, detachedStarts, and which trigger (upload /
  linkPreviews / relaySideEffects) fired the second revalidation. The
  summary emits from the `finally`, so a send that bails early still
  reports the steps it reached.
- New `send_perf.rs`: `log` + a `Phase` stopwatch, plus a `log_send_perf`
  command that mirrors the frontend summary into the same stderr stream.
  WKWebView drops console output logged before a Web Inspector attaches,
  and `tracing::info!` is silent here (the desktop binary installs no
  subscriber), so a plain `just dev` terminal is the only place both
  halves of one send can be read in order. A missing `[send-perf]` line
  also proves the running app predates this build.
- Backend phases, same `buzz-desktop: send-perf:` prefix: the directory
  rebuild (relay-self lookup, membership query + page count, concurrent
  10100/kind-0 batches, managed-policy batch, and whether the caller was
  the send path or a full autocomplete rebuild); NIP-11 `self` cache hit
  vs miss with the fetch cost, which directly verifies that cache; the
  send command split into event build vs submit; the shared submit split
  into rate-limiter wait vs HTTP round trip — the limiter being the one
  hidden sleeper nothing previously exposed; and `start_managed_agent`
  entry with its replay floor, so the stream shows the detached wake
  landing after the publish.
- `detachedStart` is routed through a counting wrapper so the summary
  reports whether the send fired an agent wake at all.
- Two Rust tests pin `Phase` resolution and monotonicity; six JS tests
  pin value passthrough, recording on a thrown step, step-name
  accumulation, fact merging, idempotent finish, and the total spanning
  its steps. The fast-path mentions spec now also pins exactly one
  `log_send_perf` per send, and the E2E bridge mocks the command.

Behavior is unchanged: the revalidation-trigger refactor evaluates the
same three conditions in the same order, and the probe swallows its own
errors so it can never fail a send.

Note: the repository file-size ratchet is red on this branch for
app_state.rs, agents.rs, managed_agents/runtime.rs and
useMentionSendFlow.ts. All four were already over before this change;
this commit keeps identity_archive.rs under the limit it would otherwise
have crossed, but does not split the pre-existing four.

Verified: cargo test --lib on desktop/src-tauri (3015 passed), clippy -D
warnings + fmt, desktop tsc --noEmit, biome, desktop unit tests (5805
passed), and the mentions + channels Playwright smoke suites (171/172 —
the one failure is the pre-existing under-load flake in the publish-first
provider-deploy spec, green 3x in isolation), plus a 3x stress rerun of
the spec carrying the new assertion.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
`spawn_agent_child` wrote BUZZ_ACP_REPLAY_FLOOR near the top of the
command build, but the fully-layered user env (`descriptor.env`) is
written last — "so user-explicit values win over Buzz-set env" — and the
key is not in RESERVED_ENV_KEYS. A persona/global/agent env entry named
BUZZ_ACP_REPLAY_FLOOR therefore overrode this send's floor, and defeated
the `env_remove` on the None path too. A saved future-dated value clamps
to `now` in `startup_watermark_with_floor`, so the harness booted blind
to the just-published mention that triggered the spawn — the precise
failure the floor exists to prevent. This is the same shadowing that
`apply_replay_floor` already strips from `launch.env` for provider
deploys and pins with `caller_replay_floor_strips_user_env_shadow`.

Fixed via the post-loop write rather than a reserved key, because it
reproduces the provider path's semantics exactly:

- New `apply_replay_floor_env` in `runtime/metadata.rs`, called after the
  `descriptor.env` loop alongside `apply_effort_env` (B5) — a caller
  floor wins over any user-supplied entry. With no caller floor the key
  is left as `descriptor.env` wrote it, matching the provider payload
  where a floorless deploy passes a user `launch.env` value through.
- The pre-loop write becomes an unconditional `env_remove`, so the
  ambient parent-process value is stripped on both paths (previously only
  the None path cleared it) before user env and the caller floor are
  layered on. `None` can no longer inherit the floor from the
  environment Desktop itself was launched with.
- Adding the key to RESERVED_ENV_KEYS was the alternative; it was not
  taken because that list is deliberately narrow and security-scoped, and
  it would additionally reject the key at env save time, diverging from
  the provider path's passthrough on a floorless deploy.

Also lifts the env var name into a shared `REPLAY_FLOOR_ENV_VAR` const
consumed by both the local spawn and `provider_deploy::apply_replay_floor`,
matching how SESSION_TITLE_ENV_VAR and EFFORT_LEVEL_ENV_VAR are named
from one place, so the two halves cannot drift.

Four unit tests pin the contract: caller floor wins over a user env
collision, user value survives with no caller floor, the ambient strip
holds when neither supplies one, and a caller floor re-asserts after the
strip. The behavior is backend-only, so the existing E2E specs pinning
`replayFloorUnix` on the invoke payload are unaffected.

The repository file-size ratchet stays red on this branch for
app_state.rs, agents.rs, managed_agents/runtime.rs and
useMentionSendFlow.ts. All four were already over before this change;
runtime.rs is unchanged at +27 lines here (the new helper and its tests
live in runtime/metadata.rs), so this commit adds nothing to the ratchet
and does not split the pre-existing four.

Verified: cargo test --lib on desktop/src-tauri (3019 passed), clippy -D
warnings on --all-targets, cargo fmt --check, and the desktop file-size
ratchet re-run to confirm runtime.rs holds at its inherited delta.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
The publish-first wake called `startManagedAgent` with no
`expectedRelayUrl`/`expectedSignerPubkey`, so `bind_expected_relay_scope(None,
…)` let the backend resolve whatever workspace relay and signing identity were
current at execution time. Awaiting the start used to bound that window to the
send; detaching it means the call outlives the send, the channel, and — since
community switching only remounts the React subtree — the community itself. A
user who mentions a stopped agent and immediately switches communities could
have it spawned or deployed against the new tenant's relay, carrying the old
community's replay floor. `submitProjectAgentMessage` passes both values for
exactly this outlives-its-caller reason.

- New `useDetachedAgentStart`: captures the active community's relay URL and
  the identity pubkey per render and passes them with every detached start, so
  `start_managed_agent` fails closed when either has moved. Both are captured
  because a switch mutates the relay and the signing keys under separate locks
  — pinning only the relay would still let the new identity act for the old
  tenant. Capture is per render, never re-read after the switch it guards
  against: a send in flight holds the callback from the render that fired it.
- The relay URL is handed over verbatim rather than through the shared
  `normalizeRelayUrl` (which lowercases for storage keys) — the backend's
  `relay_http_base_url` comparison is case-sensitive past the scheme, so a
  stored `wss://Relay.Example` would otherwise mismatch forever and refuse
  every wake. The signer check is case-insensitive, so that side is
  canonicalized.
- Scope-mismatch failures get their own toast detail. The backend's message
  ends in "not sent", which is wrong here: publish-first means the message did
  publish and only the wake was refused.
- `startAgentDetached` moves out of `useMentionSendFlow.ts` into the new hook,
  which also drops that file by 21 lines.

Two unit tests drive the real hook under the real `CommunitiesProvider`: one
pins the scope (and the surviving replay floor) on a normal start, using a
mixed-case relay URL to pin the verbatim handoff; the other captures the
callback, switches community, and asserts the stale wake still names the
community it was fired in. Both fail without the fix. The in-channel
publish-first E2E spec now also pins both scope fields on the detached invoke,
and a new spec moves the active community while the start is held behind an
injected delay and asserts the message stays published while the wake is
refused with the reworded toast.

The repository file-size ratchet stays red on this branch for app_state.rs,
agents.rs, managed_agents/runtime.rs and useMentionSendFlow.ts. All four were
already over before this change; this commit shrinks useMentionSendFlow.ts
(1098 -> 1077) and adds nothing to the other three.

Verified: desktop tsc --noEmit, biome check over src + tests, desktop unit
tests (5807 passed; the two new tests re-run green after the final verbatim-
relay-URL edit), and
the full mentions (84) and channels (89) Playwright smoke suites against a
pnpm build:e2e bundle, plus a 3x rerun of the new fail-closed spec. Both new
E2E assertions were confirmed to fail with the scope threading removed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
`applyReusableAgentAccessPolicy` signalled "this hit the relay" by
returning a different object than it was handed: a matching policy
returned the caller's `agent`, a diverging one returned the fresh record
from `updateManagedAgent`. `useMentionSendFlow` read that with
`readyAgent !== agent` to decide whether an awaited relay round-trip
separated its pre-side-effect mention-authorization pass from the
publish, and therefore whether to revalidate at the publish boundary
(#5681).

The contract held today, and the safe failure direction (a gratuitous new
object) only costs a redundant pass — but the unsafe direction is silent:
a future in-place cache update that writes to the relay and returns the
caller's object would skip the publish-boundary revalidation, and nothing
pinned the convention. Make the signal load-bearing by construction.

- `applyReusableAgentAccessPolicy` now returns
  `{ agent, wrote }` (`ApplyReusableAgentAccessPolicyResult`), with
  `wrote` set from whether `updateManagedAgent` actually ran rather than
  from anything about the returned record. Its doc comment names the
  send-path consumer so the flag is not mistaken for incidental.
- The send path destructures `{ agent: readyAgent, wrote }` and sets
  `wroteRelayState` from `wrote`; the existing-member branch supplies
  `{ agent, wrote: false }`. Behaviour is identical — for the current
  implementation the two signals agree case for case.
- `provisionChannelManagedAgent`'s two reuse branches destructure the
  agent out; they never consulted the identity signal.

Three unit tests pin the contract in a new
`channelAgents.accessPolicy.test.mjs`: a matching policy reports
`wrote: false`, returns the input agent, and issues no command; a
diverging policy reports `wrote: true` and invokes
`update_managed_agent` with the resolved policy; and a write whose
response is content-identical to the input still reports `wrote: true`,
which is the case an identity or content comparison would miss.

The repository file-size ratchet stays red on this branch for
app_state.rs, agents.rs, managed_agents/runtime.rs and
useMentionSendFlow.ts. All four were already over before this change;
useMentionSendFlow.ts holds at its inherited 1077 lines here, so this
commit adds nothing to the ratchet and does not split the pre-existing
four.

Verified: desktop tsc --noEmit, biome check over src + tests (exit 0),
desktop unit tests (5810 tests, 5804 passed; the 5 failures are the
pre-existing inboxReopenNavigation and useRetainedProjectGitViews
loader failures, present on origin/main), and the full mentions (84) and
channels + agent-access-warning (93) Playwright smoke suites against a
pnpm build:e2e bundle.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Awaiting the start used to make a duplicate wake unreachable, via three
things stacked up: the start ran inside `completeSend`; the mutation's
`isPending` fed `isPreparingMentionSend`, a hard early return in the
composer's send handler, so *no* send could begin while one was in
flight; and by the time that gate lifted `onSuccess` had written the
running/deployed record into the query cache, so the next send read
`running` and never re-fired.

Publish-first removed the first two by design, and that breaks the
third: the cache is only updated on success, so for the whole in-flight
window `getManagedAgentsByPubkey` keeps returning the stale `stopped` /
`not_deployed` record and `ensureManagedAgentMentionsReady` fires again.
The window is longest exactly where it matters — a cold local spawn or a
first remote deploy is seconds long. Reachable by two quick sends in one
composer (`@fizz do X` → `@fizz also Y`), or by two composers at once
(channel, thread panel, `NewMessageScreen` each hold their own
`useMentionSendFlow`).

Implements option A: a module-level in-flight map in
`useDetachedAgentStart`, the single callback every path funnels through
(including the `attachAgentMutation` and persona-create paths that take
it as `detachedStart`).

- Keyed by `(scoped relay URL, normalized pubkey)`, mirroring the
  backend's own runtime pair key, so a wake in one community can never
  suppress one in another — the same tenant boundary `expectedRelayUrl`
  already enforces. Module-level rather than a ref because the
  cross-composer overlap is one of the two cases worth collapsing; a ref
  dies on remount and would miss it.
- No synchronisation is needed or possible: JS is run-to-completion and
  the check, the call and the registration sit in one synchronous block
  with no `await` between them. The real hazards are elsewhere — the
  `delete` is in `finally`, not `then`, so a failed start does not latch
  the agent for the session, and it is identity-guarded so an A→B→A
  switch (where `resetDetachedAgentStarts` cleared the map and a newer
  start re-registered the key) cannot drop the newer entry.
- Suppressing rather than queueing a re-run is correct because the wake
  is per-agent, not per-message: the first start's replay floor predates
  the second message and the floor is a lower bound, so one harness boot
  covers both. It also collapses two failure toasts into one, whose
  wording ("your message was sent, but the agent may not respond") is
  accurate for both messages.
- `resetDetachedAgentStarts()` is registered in `resetCommunityState()`
  per the module-singleton rule, and is semantically right: those starts
  belong to the community being left and would fail closed at the
  backend's scope assertion anyway.
- The callback now returns whether it fired, and `countDetachedStart`
  counts only real fires — otherwise the send-perf summary this branch
  added for attributing latency would report wakes that never happened.

What this does not cover: the Agents-panel Start button, the restore
path, and inbound-persona deploys all call `startManagedAgent` outside
this hook. Closing that class needs backend coalescing (a per-pubkey
deploy epoch snapshotted before the deploy lock, so an explicit redeploy
still forces) — deferred as its own change. One residual on the covered
path: if a second send updates a provider agent's access policy while
the first deploy is in flight, the suppressed deploy would have carried
the newer policy; `apply_deploy_result` only clears
`provider_policy_pending` on a matching payload and
`reconcile_on_workspace_apply` retries pending records, so it self-heals
at the next workspace apply, just later.

Five unit tests pin the contract, each confirmed to fail under the
mutation it guards: a second wake for the same agent is suppressed while
the first is held open (fails with the map removed); different agents in
one window are not collapsed (fails with a relay-only key); the same
agent in another community is not suppressed (fails with a pubkey-only
key); and a wake fires again after the previous one resolves *or*
rejects (both fail without the `finally` delete). A new E2E spec sends
twice with the same mention behind the 45s `startManagedAgentDelayMs`
and asserts both messages publish while `start_managed_agent` stays at
exactly one call — also confirmed to fail (2 vs 1) with the map removed.

The repository file-size ratchet stays red on this branch for
app_state.rs, agents.rs, managed_agents/runtime.rs and
useMentionSendFlow.ts. All four were already over before this change;
useMentionSendFlow.ts holds at its inherited 1077 lines here, so this
commit adds nothing to the ratchet and does not split the pre-existing
four.

Verified: desktop tsc --noEmit, biome check over src + tests (exit 0),
desktop unit tests (5815 passed, 0 failed), and the full mentions (85)
and channels (89) Playwright smoke suites against a pnpm build:e2e
bundle — 2 mentions specs failed under load and are green in isolation
(the inline-code-span literal and system-agent-avatar-stack specs, both
untouched here), plus a 3x stress rerun of the new spec.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
`useDetachedAgentStart` fell back to `undefined` for both halves of its
tenant scope when the active community or the identity query had not
resolved. The backend reads a missing `expectedRelayUrl` /
`expectedSignerPubkey` as "no assertion" (`assert_expected_relay_scope`
returns `Ok(())` for `None`), so in that window the wake fired fully
unscoped — silently degrading the fail-closed guarantee the hook exists
to provide, and collapsing its dedupe key to a relay-less one shared
across communities. A blank stored relay URL had the same effect: the
backend discards a whitespace-only scope exactly as it discards a
missing one, and only `""` was screened out here.

Refuse the wake instead of firing it:

- Both scope values are checked before anything else in the callback; a
  missing one returns `false` (so the send-perf summary counts no wake)
  and raises the toast. Emptiness is now judged on the trimmed relay
  URL, matching the backend, while the value still reaches it verbatim
  so the case-sensitive comparison holds.
- Deferring until the query resolves was the alternative and is wrong
  here: reading the scope after the send is exactly the post-switch
  re-read the per-render capture rules out. Refusing is recoverable —
  the user's next send re-fires the wake.
- The refusal toast reuses the failure path's wording via a shared
  `warnAgentMayNotRespond`, because publish-first means the message went
  out in both cases and the user is owed the same warning; only the
  trailing detail differs ("Buzz is still connecting to this community
  — mention the agent again in a moment.").

Three unit tests pin it, each confirmed red without the guard: a wake
fired while `get_identity` is held open makes no `start_managed_agent`
call, reports that it fired nothing, warns the user, and still fires
fully scoped once the query lands; a wake with no active community is
refused; a whitespace-only relay URL is refused rather than passed as no
assertion.

The harness's "wait one tick for the identity query" also became
load-bearing with this change — an under-wait now reads as a phantom
suppression bug rather than passing on an unscoped call — so
`renderDetachedStart` waits on the rendered identity value instead. That
under-wait was already happening: the existing dedupe test fails against
the fix until the harness waits properly.

The repository file-size ratchet stays red on this branch for
app_state.rs, agents.rs, managed_agents/runtime.rs and
useMentionSendFlow.ts. All four were already over before this change;
this commit touches none of them.

Verified: desktop tsc --noEmit, biome check over src + tests, desktop
unit tests (5818 passed, 0 failed), and the mentions (86) and channels
(89) Playwright smoke suites against a pnpm build:e2e bundle.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…tion-2

Signed-off-by: Matt Toohey <contact@matttoohey.com>
The send-path work pushed four files past the repository file-size gate.
Extract one cohesive unit from each rather than raising a ceiling:

- `runtime.rs` → `runtime/setup_payload.rs`: the readiness → payload JSON →
  `BUZZ_ACP_SETUP_PAYLOAD` write, as `apply_setup_payload_env`.
- `commands/agents.rs` → `commands/agents_create_fields.rs`: the pure
  create-request field validators/resolvers.
- `app_state.rs` → `app_state_accessors.rs`: the inherent `AppState`
  lock accessors, leaving the struct, builder, and identity resolution.
- `useMentionSendFlow.ts` → `useEnsureAgentMentionsReady.ts`: the mentioned
  agent access-policy / membership / detached-wake reconciliation.

Pure moves. The one behavioral difference is that the new hook depends on
the stable `mutateAsync` rather than the whole mutation object, so
`ensureManagedAgentMentionsReady` no longer changes identity every render.

Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e requested a review from a team as a code owner September 1, 2026 01:58
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is 571c1902d0ca55cfd4ccf6b91eeb731909cc10be...d7e6e3ec862ea76dc96fbdd2493b63b08015f324.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review d7e6e3ec862ea76dc96fbdd2493b63b08015f324 to authorize a new review.
Any previous review applies only to its recorded range.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — REQUEST CHANGES at exact head 00de55e019fd65ccc9026981e35a79872d758e19 (base 4a9de1a3a121285ef475d630b2b5764044c02cde).

Risk: high — this changes the authorization and lifecycle ordering of relay publication, managed-agent deployment, and cross-community UI failure delivery.

Blocking findings

  1. Publish-boundary mention authorization can go stale across unbounded awaited work. desktop/src/features/messages/ui/useMentionSendFlow.ts:378-394 admits mention pubkeys before managed-agent lookup; readiness/persona work and sync_agents_to_active_huddle then suspend at :436-479. The second authorization pass at :531-543 is conditional on upload/link work or a command reporting relay side effects, so a slow lookup, store lock, provider lookup, or no-active-huddle IPC can span membership revocation and still publish the previously admitted p tag. The base path revalidated unconditionally.

    Author action: retain the latency optimization without weakening freshness: place authorization immediately before a provably suspension-free publish boundary, or revalidate whenever awaited work follows admission. Add a causal regression that holds a no-relay-write/no-active-huddle leg, revokes membership, and proves the outgoing p tag is stripped; mutation-disable the second pass and require failure.

  2. The implementation can claim “your message was sent” before publication, including when publication fails. completeSend awaits readiness at useMentionSendFlow.ts:436-445; readiness starts the detached agent at desktop/src/features/messages/ui/useEnsureAgentMentionsReady.ts:116-131; relay publish does not begin until useMentionSendFlow.ts:552-561. An immediate start rejection can therefore emit the success-worded warning from desktop/src/features/agents/hooks/useDetachedAgentStart.ts:50-59,144-152 before relay acceptance. If publish then rejects, the composer is restored at useMentionSendFlow.ts:627-635, but the false success claim remains and an agent may have been woken for a message the relay never accepted.

    Author action: queue the scoped wake and fire it only after successful relay publication, or otherwise condition both wake and warning on confirmed acceptance. Add deterministic wake-rejection + publish-rejection coverage proving no “message was sent” warning and no orphan wake.

  3. A stale wake failure can leak community A’s agent context into community B. Community reset clears only the dedupe map (desktop/src/features/community/hooks/useCommunityInit.ts:55-85); it neither cancels nor fences the detached promise. Its catch still toasts A’s agent name/backend detail from useDetachedAgentStart.ts:50-59,144-152, while the global toaster remains mounted across the switch (desktop/src/main.tsx:81-105). The new E2E around desktop/tests/e2e/mentions.spec.ts:2797 expects the stale post-switch warning and mutates storage rather than exercising the real community transition.

    Author action: bind warning delivery to the captured community and signer, suppress UI delivery after either changes, and add a real A→B UI-transition regression proving no A agent name or startup detail appears in B.

  4. Clearing tenant-keyed in-flight dedupe on every switch permits duplicate remote deployment after A→B→A. The map is already scope-keyed (useDetachedAgentStart.ts:26,131), but useCommunityInit.ts:55-83 clears A’s still-running entry. Returning to A allows another send to queue a second deployment while the first is active. The backend per-pubkey mutex serializes rather than coalesces this work, and already-deployed agents may be deployed again (desktop/src-tauri/src/commands/provider_deploy.rs:19-23,54-65), potentially replacing the harness that just came up for the first message.

    Author action: preserve safely scope-keyed in-flight entries across community reset, or coalesce them behind a scope-aware backend epoch. Add A→B→A coverage with deployment #1 held: the second A send must not invoke or queue deployment #2, while success/rejection must still unlock a later retry.

Verification owner: author for the four deterministic regressions and fixes; reviewer for exact-head rerun and ordering/ownership trace.

Validation at the reviewed head

  • Full Desktop JS package: 5,856/5,856 passed in both assigned lanes.
  • git diff --check 4a9de1a...00de55e: passed.
  • Full cargo test -p buzz-acp: 903 passed / 1 timing failure (keepalive_resets_idle_past_deadline at 111 ms); isolated rerun passed. No causal link to this PR was established.
  • A separate full smoke attempt hit the 600-second reviewer harness timeout after 374 displayed passes with no test failure shown.
  • The full local Tauri package was not completed because the fresh worktree lacked desktop/src-tauri/binaries/buzz-acp-aarch64-apple-darwin.

Confidence gaps: native Desktop/AX behavior and full Tauri completion remain reviewer/tooling-owned; hosted Desktop Core, one smoke shard, and integration shards were still running at the latest lane snapshots. These are not additional author defects. Source review found no concrete content/privacy leak in the new performance instrumentation.

Any new head invalidates this verdict.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

REQUEST CHANGES on 00de55e019fd65ccc9026981e35a79872d758e19 against 4a9de1a3a121285ef475d630b2b5764044c02cde.

I found four author-actionable races in the publish-first lifecycle:

  1. [P1] The UI can claim “your message was sent” before relay acceptance, and even when publish fails. completeSend awaits readiness at desktop/src/features/messages/ui/useMentionSendFlow.ts:436-445; readiness immediately invokes detached start at useEnsureAgentMentionsReady.ts:116-131; relay publish begins only later at useMentionSendFlow.ts:552-561. An immediate wake rejection therefore reaches the unconditional success-worded toast at useDetachedAgentStart.ts:50-59,144-152 before relay acceptance. If publish then rejects, useMentionSendFlow.ts:627-635 restores the composer, but the user has already received the false success claim, and an agent may have been woken for a message the relay never accepted. Required: queue the scoped wake and fire it only after successful publish (or equivalently gate wake and warning on confirmed acceptance), plus a deterministic wake-rejection + publish-rejection regression asserting no success warning and no orphan wake.

  2. [P1] A detached failure from community A can disclose A’s agent context after switching to B. Community teardown only clears dedupe state (useCommunityInit.ts:55-85); it neither cancels the promise nor fences its UI. The catch still displays A’s agent name/backend detail (useDetachedAgentStart.ts:50-59,144-152), while the global toaster survives the switch (desktop/src/main.tsx:81-105). The added test at desktop/tests/e2e/mentions.spec.ts:2797+ expects the stale warning and mutates storage rather than exercising the real switch. Required: bind warning delivery to captured community + signer, suppress it once either changes (logging may remain), and add a real A→B UI regression proving no A identity/detail appears in B.

  3. [P2] The fast path reuses stale mention authorization across awaited work. The first pass admits pubkeys at useMentionSendFlow.ts:378-394, before awaited managed-agent lookup, readiness/persona work (:436-445), and sync_agents_to_active_huddle (:469-479). The second pass runs only for upload, previews, or a command reporting relay work (:531-543). A slow cache/store/provider/no-active-huddle leg can therefore suspend while membership is revoked, yet publish with the old admitted set. The base path revalidated unconditionally. Required: ensure authorization is fresh at the publish boundary—move admission after all suspending work or revalidate whenever awaited work follows it—and add a causal delayed no-relay-write/no-active-huddle regression that revokes membership and requires the outgoing p tag to be stripped.

  4. [P2] Clearing tenant-keyed in-flight dedupe on every switch permits duplicate A deployments on A→B→A. The map is already scope-keyed (useDetachedAgentStart.ts:26,131), but useCommunityInit.ts:55-83 clears all entries. Returning to A while its first start is running permits another start. Backend provider_deploy.rs:54-65 serializes but does not coalesce; :19-23 deliberately redeploys an already-deployed agent, so deploy #2 can replace the harness that just came up for message #1. Required: retain safely scoped in-flight entries across switches (or coalesce in a scope-aware backend epoch), with an A→B→A held-deploy regression proving no second deployment and proving later retry unlocks after completion/rejection.

Verification at this exact head

  • PASS: git diff --check 4a9de1a...00de55e; clean detached worktrees.
  • PASS: full Desktop JS package, 5,856/5,856.
  • cargo test -p buzz-acp: 903 passed, 1 failed (keepalive_resets_idle_past_deadline, 111 ms); isolated exact-head rerun passed. Treated as a non-PR flake.
  • Full Desktop smoke locally timed out at the 600s harness limit after 374 displayed passes with no observed failure; confidence gap, not requested author rework.
  • Full Tauri local package was blocked by the absent desktop/src-tauri/binaries/buzz-acp-aarch64-apple-darwin; confidence gap, not requested author rework.
  • CI at final freshness check remained in progress; completed checks included Unit Tests, relay/backend integration, macOS/Windows builds, Rust lint, security, Semgrep, and DCO.
  • Native Desktop/AX observation was not run. Reviewer/tooling-owned confidence gap; the deterministic lifecycle regressions above remain author-owned.

No additional instrumentation privacy defect was found: the new perf record contains counts/flags/timings rather than content, pubkeys, channel IDs, or relay URLs (sendPerfLog.ts:24-31,54-61). NIP-11 cache scoping/TTL/failure behavior, replay-floor handling, and local/provider scope rechecks appeared coherent in the reviewed source and focused coverage.

The send-perf probes added in adc7330 existed to attribute the
post-publish-first spinner latency, and that analysis is done — the run
log confirmed the shipped fixes and named the residual cost. Remove the
instrumentation so the send path carries no per-send probe; every
behavior it measured (publish-first wake, replay floor, revalidation
dedupe, NIP-11 self cache) stays.

- Frontend: delete `sendPerfLog.ts` and its six tests. `completeSend` no
  longer wraps its awaited steps or records facts; the publish-boundary
  revalidation returns to the direct three-condition ternary it wrapped
  (same conditions, same order). The perf-only `detachedStarts` counter
  leaves `EnsureAgentMentionsReadyResult` — `startAgentDetached` is
  passed straight through again. Its boolean "did this fire" return
  stays: the dedupe and fail-closed unit tests pin it; only the doc
  wording that justified it via the send-perf summary is gone.
- Backend: delete `send_perf.rs` (the `Phase` stopwatch, its two tests,
  and the `log_send_perf` mirror command) and strip the phase timers and
  log lines from the relay-directory rebuild (`query_all_relay_pages`
  returns just the events again), the NIP-11 `self` cache hit/miss path,
  `send_channel_message`'s build/submit split, the shared submit's
  rate-limiter/HTTP split, and `start_managed_agent` entry.
- E2E: drop the `log_send_perf` bridge mock and the one-summary-per-send
  assertion from the fast-path mentions spec; its revalidation-count
  assertions remain and still pin the restored ternary.

Verified: cargo fmt --check, clippy --all-targets -D warnings, and cargo
test --lib on desktop/src-tauri (3054 passed — down exactly the two
removed Phase tests), desktop tsc --noEmit, biome check over src + tests
(exit 0), desktop unit tests (5850 passed, 0 failed — down exactly the
six removed sendPerfLog tests), and the modified mentions spec green
against a fresh pnpm build:e2e bundle.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

REQUEST CHANGES on exact head 7afc7eee61043e17e31b9c4811e9aa6666901d55 against base 4a9de1a3a121285ef475d630b2b5764044c02cde.

The new commit removes performance instrumentation but leaves four current-head lifecycle defects intact.

Blocking findings

  1. [P1] A wake failure can claim “your message was sent” before relay acceptance, including when publish fails. desktop/src/features/messages/ui/useMentionSendFlow.ts:418-425 awaits readiness; readiness fires detached start at useEnsureAgentMentionsReady.ts:105-120; relay publication occurs later at useMentionSendFlow.ts:516-523. The detached rejection path unconditionally emits the success-worded warning at useDetachedAgentStart.ts:56-59,149-151. If publication then rejects, useMentionSendFlow.ts:589-593 restores the composer, but the false claim remains and an agent may have been woken for an unaccepted message.

    Author action: queue scoped wake intents and fire only after successful relay publication, or otherwise condition both wake and warning on confirmed acceptance. Add deterministic start-reject + publish-reject coverage proving no success warning and no orphan wake.

  2. [P1] A delayed community-A wake failure can disclose A’s agent context while B is active. The detached catch still toasts the captured A agent name/error without checking active community or signer (useDetachedAgentStart.ts:149-151). Community reset only clears the module map (:28-35; features/communities/useCommunityInit.ts:55-82), not the outstanding promise or global toast. The retained E2E at desktop/tests/e2e/mentions.spec.ts:2791-2865 mutates local storage and expects the stale warning rather than driving A→B and proving suppression.

    Author action: fence warning delivery to the captured community and signer, suppressing UI output after either changes, and add a real rail-driven A→B regression proving no A agent identity or startup detail appears in B.

  3. [P2] Mention authorization can remain stale across awaited work before publish. Admission occurs at useMentionSendFlow.ts:367-369, before managed-agent lookup (:378), readiness/persona work (:418-425), and huddle IPC (:445-453). The second pass remains conditional on upload, previews, or a boolean reporting relay side effects (:496-507). A no-write/no-active-huddle leg can therefore suspend while membership is revoked and still publish the old admittedMentionPubkeys at :516-523. The current fast-path test explicitly expects only one revalidation (desktop/tests/e2e/mentions.spec.ts:1854-1860).

    Author action: revalidate after any post-admission suspension or move admission to a genuinely immediate publish boundary. Add a held no-write/no-active-huddle regression that revokes membership and proves the outgoing p tag is stripped.

  4. [P2] Community reset destroys tenant-keyed in-flight coalescing and permits duplicate provider deployment after A→B→A. The map is already keyed by relay URL and pubkey (useDetachedAgentStart.ts:26,130), but reset clears every entry (:29-35; useCommunityInit.ts:55-82). Returning to A while its first start is unresolved can invoke a second start. Native provider locking only serializes (desktop/src-tauri/src/commands/agents/provider_deploy.rs:54-65); it does not coalesce, and deployed records are deliberately redeployed (:19-23).

    Author action: retain safely tenant-keyed in-flight entries across switches, or coalesce through a scope-aware backend epoch. Add held-deploy A→B→A coverage proving one A deployment while settlement still unlocks later retry.

Verification owner: author for all four fixes and causal regressions; reviewer for exact-head ordering/ownership re-trace.

Exact-head validation

  • Full Desktop JS package: 5,850/5,850 passed in both lanes.
  • Desktop TypeScript typecheck and lint: passed in the product lane.
  • Focused changed-journey smoke tests: 3/3 passed (publish-first stopped agent, detached failure warning, retained pseudo-switch test).
  • git diff --check 4a9de1a...7afc7eee: passed.
  • Delta 00de55e...7afc7eee: 15 files, 39 insertions / 457 deletions; instrumentation/timer/counter removal introduced no additional concrete defect found in the inspected delta.
  • Live head was refreshed immediately before submission and remained 7afc7eee61043e17e31b9c4811e9aa6666901d55; GitHub user jedwards27 differs from author matt2e.

Confidence gaps: native Desktop/AX observation, full local Tauri/native package, and full local smoke completion were not rerun for this round. Several hosted jobs remain in progress. These are reviewer/tooling or CI-owned confidence gaps, not additional author rework.

Any new head invalidates this verdict.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of base 4a9de1a3a121285ef475d630b2b5764044c02cde → exact head 7afc7eee61043e17e31b9c4811e9aa6666901d55.

The latest commit removes send-performance instrumentation, but the current production paths still contain four author-actionable ordering/ownership defects:

  1. [P1] A detached wake failure can claim the message was sent before relay publish succeeds. useMentionSendFlow.ts:418-425 awaits readiness before the relay send at :516-523; readiness starts the detached agent at useEnsureAgentMentionsReady.ts:105-120; and useDetachedAgentStart.ts:149-151 emits the global “your message was sent” warning on rejection. An immediate start rejection can therefore display success wording before acceptance, and if publish then rejects (useMentionSendFlow.ts:589-593) the composer is restored while the false success claim remains. It also wakes an agent for a message that may never be accepted. Queue the scoped wake until successful publish (or otherwise gate wake and warning on confirmed publish), with a deterministic start-reject + publish-reject regression proving no success wording and no orphan wake.

  2. [P1] A delayed community-A wake failure can leak A’s agent context after switching to B. The detached promise catch globally toasts its captured agent name/error (useDetachedAgentStart.ts:149-151) without checking the active community or signer. resetDetachedAgentStarts() (:28-35) clears bookkeeping, not the outstanding promise or globally mounted toaster. The retained test at desktop/tests/e2e/mentions.spec.ts:2791-2865 mutates localStorage and positively expects the stale warning rather than performing the real rail switch and proving suppression. Fence warning delivery to the captured community and signer, and add a real A→B regression asserting that A’s agent name/backend detail never appears in B.

  3. [P2] Mention authorization can be stale at publish after awaited “no side effect” work. Admission occurs at useMentionSendFlow.ts:367-369, then managed-agent lookup, readiness/persona work, and huddle IPC can suspend execution (:378,418-425,445-453). Revalidation at :496-507 is conditional on upload, previews, or a relay-side-effect boolean. A matching access policy (wrote=false) plus no active huddle leaves that boolean false, so revocation during the suspension still publishes the old admittedMentionPubkeys at :516-523. Revalidate after every post-admission suspension, or move admission to the immediate publish boundary; add a held no-write/no-active-huddle revocation test.

  4. [P2] Community reset breaks tenant-keyed in-flight coalescing, permitting duplicate A provider deploys on A→B→A. The start map is already keyed by relay URL and pubkey (useDetachedAgentStart.ts:26,130), but global reset clears every tenant (:29-35) and community reset calls it from useCommunityInit.ts:55-82. Returning to A while A’s first start is unresolved can issue a second deploy. Native locking in provider_deploy.rs:54-65 serializes but does not coalesce, and deployed records are intentionally redeployed (:19-23). Retain tenant-keyed in-flight entries across switches (or coalesce with a scope-aware backend epoch), with a held A deploy → B → A test requiring one A invocation and settlement still permitting retry.

Exact-head evidence:

  • Full Desktop JS package: 5,850 passed, 0 failed.
  • Desktop TypeScript typecheck and lint passed (lint retained unrelated warnings/info).
  • E2E build plus focused publish-first, detached-failure-toast, and current pseudo-switch smoke tests passed.
  • git diff --check 4a9de1a3...7afc7eee passed.
  • Live GitHub head was rechecked as 7afc7eee61043e17e31b9c4811e9aa6666901d55; reviewer worktrees were clean.

Confidence gaps, not additional author actions: no native GUI/AX journey or full local smoke suite was run. Several CI jobs remained in progress at submission time. The instrumentation-removal delta introduced no separate user-visible or privacy defect found in the reviewed paths.

matt2e and others added 3 commits September 1, 2026 15:35
PR #7154 review point 1 (P1): the publish-first detached wake fired
during send preparation, so a fast start rejection could toast "Could
not start {agent} - your message was sent..." before the relay had
accepted anything, and when the publish then failed the composer
silently restored with that false claim as the only messaging on
screen. The fail-closed scope refusal toasted the same wording
synchronously, deterministically pre-publish. Any abort after the wake
(cancel, readiness error, huddle failure, publish rejection, dismissed
non-member prompt) also stranded a started harness for a message that
never landed.

Implement the reviewer's fix (option A of the analysis): wakes queue
while the send prepares and flush fire-and-forget only after
`await send(...)` resolves.

- useEnsureAgentMentionsReady no longer takes startAgentDetached; it
  returns `agentsToWake` - the member fast path enqueues directly, the
  attach path hands the collector to attachManagedAgentToChannel as
  `detachedStart` (whose signature is unchanged).
- createMentionedPersonaAgents collects the same way and carries its
  entries on the pending draft, which also moves persona wakes behind
  the non-member prompt: a dismissed prompt now drops them instead of
  waking an agent for a message the user chose not to send.
- completeSend merges the draft and readiness queues (first entry per
  agent wins - it carries the earliest floor) and finishSend flushes
  them immediately after the publish resolves, before the post-send
  cancellation check, so a cancellation racing a successful publish
  cannot drop the wake for a message that did land. The spinner gains
  no awaited work; every abort path simply drops the queue.
- Each wake's replay floor is captured at enqueue time, not flush time:
  the flush now runs post-publish, so a flush-time stamp could exceed
  the message's created_at and push the harness's startup watermark
  past the very message the floor exists to cover (buzz-acp clamps the
  floor to [now-15min, now]). useDetachedAgentStart takes the queued
  floor as an optional parameter and keeps fire-time capture for
  callers without one. The toast wording is untouched - "your message
  was sent" is now structurally true whenever it can appear.

Tests, each confirmed red on the pre-fix code:
- Three useEnsureAgentMentionsReady unit tests pin the queue contract:
  a stopped member agent lands in agentsToWake with an enqueue-time
  floor and no backend call; running/deployed agents are not queued;
  the attach seam queues instead of firing.
- Two useDetachedAgentStart unit tests pin that an explicit floor
  reaches the start payload verbatim and that the no-argument form
  still captures fire time.
- New deterministic E2E spec: with sendMessageErrors and
  startManagedAgentErrors both armed, a failed publish restores the
  draft with zero start_managed_agent calls, no timeline row, and no
  "your message was sent" toast (pre-fix: one start plus the false
  toast, no timing games needed).
- The in-channel publish-first spec now also pins sign_event strictly
  before start_managed_agent.

The repository file-size ratchet stays green: useMentionSendFlow.ts
lands at 975 lines after the queue plumbing.

Verified: desktop tsc --noEmit, biome check over src + tests (exit 0),
desktop unit tests (5855 passed, 0 failed), the full mentions (86) and
channels (89) Playwright smoke suites against a pnpm build:e2e bundle,
and a 3x stress rerun of the new spec. The existing publish-first,
dedupe, fail-closed-scope, and start-failure-toast specs all stay
green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…munity

PR #7154 review point 2: <Toaster /> mounts outside the community
remount boundary, and the detached wake's catch toasted its captured
agent name and error detail unconditionally — so a start that settled
after a community switch rendered community A's failure over community
B's UI. The wake itself already fails closed at the backend; what leaked
was warning delivery: a toast in B naming an agent that does not exist
in B, carrying an uncontextualized backend error, reads as a bug in B.

Implement the reviewer's fix as designed in the review analysis — a
module-level scope mirror compared at toast time:

- New detachedToastScope module mirrors the on-screen tenant scope
  outside React: set to {relayUrl, signerPubkey} by useCommunityInit
  when a community apply completes, cleared in resetCommunityState()
  per the module-singleton rule.
- The catch in useDetachedAgentStart delivers the failure toast only
  when the scope captured at fire time matches the mirror — the relay
  URL verbatim past a trim and the signer case-insensitively, the
  backend's own comparison semantics. Mismatch or no mirror suppresses
  with a console.warn for diagnosability rather than rewording: any
  wording still names another community's agent over B's UI.
- Scope comparison, deliberately not a reset generation: an A→B→A
  round-trip restores A's mirror, so a slow start fired in A still
  warns once the user is back in A — exactly where "mention the agent
  again" is actionable. A generation check would silently drop it.
- The synchronous unscoped-start refusal toast stays unfenced by
  design: it fires at send time in the firing community.

Tests:
- Four unit tests drive the mirror through its module seam (standing in
  for useCommunityInit, its only production writer): on-scope delivery
  as the over-suppression control, suppression after a switch plus the
  console.warn, suppression mid-switch on a cleared mirror, and A→B→A
  re-delivery. Both suppression tests are red without the fence.
- The old stale-toast E2E spec — which mutated localStorage without a
  real switch and positively pinned the stale toast, the outcome the
  fence now forbids — is replaced by a real rail-switch regression: two
  seeded communities, a genuine community-rail-button click driving the
  provider → remount → resetCommunityState path, the held start refused
  by the mock's scope check, and the toast asserted absent in B via
  non-retrying count snapshots (a retrying toHaveCount(0) waits out
  sonner's auto-dismiss and passes against the very toast it forbids —
  the first draft did exactly that and stayed green on the unfenced
  build). It keeps the published-message and scoped-invoke-payload
  assertions the unit tests cannot pin, and is red without the fence at
  the stale-toast snapshot. The existing no-switch start-failure spec
  is the E2E positive control — it now also proves the mirror is set on
  community apply.
- The mock bridge pushes a start_managed_agent:settled marker into the
  command log on resolve and reject (a distinct string, so exact-match
  commandCount("start_managed_agent") is untouched), giving the spec a
  deterministic wait for the delayed rejection before its negative
  assertions.

Verified: desktop tsc --noEmit, biome check over src + tests (exit 0,
remaining diagnostics pre-exist in unrelated files), desktop unit tests
(5859 passed, 0 failed), the full mentions (86) and community-rail (25)
Playwright smoke suites against a pnpm build:e2e bundle, a 3x rerun of
the new spec, and the repository file-size gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
… switches

PR #7154 review point 4 (P2): resetCommunityState cleared the in-flight
detached-start map on every community switch, and the justification —
"those starts belong to the community being left and are about to fail
closed at the backend's scope assertion anyway" — is wrong for a round
trip. The scope assertion is a current-state check, so a deploy still
held from community A becomes valid again the moment A is re-applied.
Nothing settles the held start across the switch (the mutation lives on
the QueryClient above the remount boundary; apply_workspace aborts no
in-flight deploys), the record still reads not_deployed for the whole
window, and the map entry was the only duplicate guard on this path —
so an A→B→A round trip plus a second mention deployed a provider agent
twice, handing the provider the second message's replay floor (past the
first message: the exact failure class this branch exists to prevent).
Local spawns were never exposed; the backend coalesces those under the
process lock.

Implement the reviewer's fix (option (a) of the analysis): retain the
entries across switches.

- resetCommunityState no longer clears the map; an explicit exemption
  comment at the call site (the canonical singleton-reset inventory)
  records why, so a future contributor does not "fix" the omission.
  Retention is safe by construction: the key IS the tenant scope
  (relay URL + pubkey), so a retained A entry can never suppress or
  affect a wake in B, and the identity-guarded finally delete
  self-cleans at settlement, which the deploy op's timeout bounds. The
  backend's own provider_deploy_locks map is likewise never cleared on
  workspace apply.
- resetDetachedAgentStarts stays exported as a test-only isolation seam
  and its doc now says so; the map header and finally-guard comments
  that encoded the refuted "fail closed anyway" rationale are rewritten
  to record the retention contract instead.
- Deliberately NOT scope-filtered clearing ("only entries for the
  community being left"): the A-scoped entry is exactly the one that
  must survive the A→B switch, so that variant recreates the bug.
- The detachedToastScope mirror reset stays: on-screen warning delivery
  is genuinely per-community state.

Out of scope, unchanged: backend deploy-epoch coalescing (option (b))
remains the deferred durable fix for the wake paths that do not funnel
through useDetachedAgentStart (Agents-panel Start, restore,
inbound-persona deploys, reconcile_on_workspace_apply). Residual
accepted: on a same-relay identity switch, a retained old-identity
entry suppresses the new identity's wake until the old start fails
closed and settles — the same accepted within-community residual,
self-resolving on the user's next send.

Tests:
- New E2E regression, the load-bearing one — it drives the real
  rail-switch → remount → resetCommunityState path that did the
  clearing: a not_deployed provider agent's deploy is held open via
  startManagedAgentDelayMs; the mention publishes and deploys once,
  scoped to A; a real A→B→A rail round-trip runs under the hold; a
  second mention back in A publishes while start_managed_agent stays at
  exactly 1; the held deploy is then settled on demand and rejects, its
  failure warning delivers back in A, and a third mention re-fires the
  wake (2 calls) — retention ends at settlement rather than latching
  the agent.
- New mock-bridge seam for that spec:
  __BUZZ_E2E_RELEASE_MANAGED_AGENT_STARTS__() settles starts held
  behind startManagedAgentDelayMs on demand, following the bridge's
  defer/release idiom — the hold must deterministically outlast the
  round-trip AND settle before the retry leg. The first settlement is
  armed to reject via startManagedAgentErrors because a successful mock
  settle writes `deployed` into the record, which would hide the retry
  leg behind the status check rather than the map — the seam under
  test.
- Two unit tests pin the complementary hook contracts in the existing
  real-CommunitiesProvider harness: an A→B→A switch hands back a
  callback whose key matches the retained entry, so the wake stays
  suppressed; and a rejection across the round-trip re-permits. The
  existing cross-community test keeps pinning that a B wake during the
  interlude is unaffected.

Verified: desktop tsc --noEmit, biome check over the touched files,
desktop unit tests (5856 passed; the 5 failures are the pre-existing
inboxReopenNavigation/useRetainedProjectGitViews baseline, present on
origin/main), the full mentions (87) and community-rail (25) Playwright
smoke suites against a fresh pnpm build:e2e bundle, a 3x stress rerun
of the new spec, and the repository file-size gate. The new E2E spec
was confirmed red on the pre-fix build — 2 deploys vs 1 at the
second-send snapshot — with the map clearing temporarily restored.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — Request changes

Reviewed base 4a9de1a3a121285ef475d630b2b5764044c02cde through exact head b4e87be877ad55a0466f2009bceeade6cab02bed.

The new head repairs the publish/wake ordering, stale-community warning disclosure, and A→B→A duplicate deployment behavior. Two author-actionable scope/freshness defects remain.

1. [P2] Mention authorization can still go stale before publication

The first mention admission at desktop/src/features/messages/ui/useMentionSendFlow.ts:379-390 precedes awaited managed-agent lookup, readiness (:430-437), and no-active-huddle IPC (:470-476). The second admission at :527-530 remains conditional on uploads, previews, or relaySideEffectsRan. A matching access policy and no active huddle can therefore leave it false despite those suspension points; membership revoked during the gap still permits the old admittedMentionPubkeys to be published at :539-546. The fast-path E2E at desktop/tests/e2e/mentions.spec.ts:1854-1860 continues to require only one revalidation rather than exercising revocation during a held no-write/no-active-huddle leg.

Author action: revalidate after every post-admission suspension, or move admission to an actually immediate publish boundary. Add a causal regression that holds a no-write/no-active-huddle operation, revokes membership, and proves the outgoing p tag is stripped; mutation-check the production trigger.

Verification owner: author for the fix/regression; reviewer for exact-head retrace.

2. [P2] Same-relay identity switches share an incomplete detached-start key

The retained in-flight key at desktop/src/features/messages/ui/useDetachedAgentStart.ts:153 contains relay URL and agent pubkey but omits the expected signer. A slow start under owner A therefore suppresses owner B's legitimate wake for the same agent pubkey on the same relay at :154-159 until A settles. Native scope is explicitly relay plus signer (desktop/src-tauri/src/commands/agents.rs:842-846), and provider deployment rechecks both after locking (desktop/src-tauri/src/commands/agents/provider_deploy.rs:27-32,90-96). The renderer must not coalesce operations belonging to distinct signer scopes. Current A/B tests retain one constant signer (desktop/src/features/messages/ui/useDetachedAgentStart.test.mjs:43,148-163).

Author action: include normalized expectedSignerPubkey in the in-flight key. Add same-relay A→B identity-switch coverage with A held: B must fire independently, the same signer must still deduplicate, and settlement must unlock retries independently per signer.

Verification owner: author for the fix/regression; reviewer for exact-head retrace.

Verified on this head

  • Full Desktop JS suite: 5,861/5,861 passed.
  • TypeScript typecheck passed; lint exited 0 with only existing unrelated diagnostics; git diff --check passed.
  • E2E build and four focused smoke journeys passed, confirming publish precedes wake, failed publish does not wake, a real A→B switch suppresses A's stale warning, and a held provider deploy survives A→B→A without duplication.
  • Hosted unit, Rust lint, relay/backend E2E, Windows/macOS builds, release-candidate, security, Semgrep, DCO, all four smoke shards, and both integration shards were successful in the final snapshots supplied by the review lanes.

Confidence gaps — not additional author actions

Native Desktop/AX and the full local smoke suite were not completed. One repeated local switch journey failed before the changed switch/wake assertions at mention-chip rendering, while the other repetitions and all hosted smoke shards passed; this did not establish a PR-caused defect. Desktop Core was still in progress in the latest lane snapshot. These remain reviewer/tooling or CI-owned verification, not reasons for additional author rework.

PR #7154 review round 2, point 2 (P2): the in-flight detached-start map
was keyed `(relay URL, agent pubkey)`, but the scope a wake actually
asserts is relay *and* signer — `start_managed_agent` runs both
`assert_expected_relay_scope` and `assert_expected_signer` before any
spawn or deploy, and the provider path re-asserts both against the
payload rebuilt after the deploy lock. So a start held under one signing
identity suppressed another identity's wake for the same agent on the
same relay: the renderer coalescing across a boundary the backend
deliberately distinguishes.

The omission came from mirroring the wrong key. The map's doc justified
the pair as matching the backend's `ManagedAgentRuntimeKey` — and it did,
that key is `(pubkey, relay_url)` with no signer. But that tracks
*runtimes*; this map tracks scoped start *operations*, and an operation's
identity is the full scope it asserts. The doc is rewritten so the next
reader does not restore the runtime-key rationale.

Reachability is narrow but real: the signing key is one global per
install, so two signers only arrive sequentially, via a mid-session
import — chiefly the membership-denied onboarding overlay, which writes
the new identity straight into the live query cache
(`queryClient.setQueryData(["identity"], …)`) while the app underneath,
including this module-level map, keeps running.

Correcting our own record from b4e87be, which accepted this as "the
retained old-identity entry suppresses the new identity's wake until the
old start fails closed and settles": that holds only if the old start had
not yet passed the scope checks. If it was already mid-spawn or
mid-deploy it does *not* fail closed — the scope was valid when checked,
the deploy runs to completion under the stale owner, and every send in
that window is dropped. Adding the signer eliminates the class rather
than shrinking the residual.

- The key is now `(relay URL, expected signer, agent pubkey)`. Relay
  stays verbatim (its backend comparison is case-sensitive past the
  scheme); the signer is already canonicalized at capture, matching
  `assert_expected_signer`'s trim + case-insensitive compare, so two
  casings of one identity cannot split the key. `undefined` can never
  reach it — the fail-closed refusal returns before key construction when
  either scope half is missing.
- Everything downstream falls out: the identity-guarded `finally` delete
  closes over the same key string, so settlement frees only its own
  signer's entry.
- Nothing existing changes. Same-signer dedupe is untouched, and the
  A→B→A community retention is untouched — the identity is global, so a
  community round-trip hands back a callback carrying the same signer,
  which still matches the retained entry.
- `resetCommunityState`'s exemption comment (the canonical
  singleton-reset inventory) now names the three-part key.

Coverage at the unit seam — the existing harness drives the real hook
under the real `CommunitiesProvider` and a real `QueryClient`, with the
identity moved by the same `setQueryData(["identity"], …)` write the
membership-denied import uses, plus a mutable `get_identity` mock so a
refetch stays consistent:

- A wake under a newly imported identity is not suppressed by one held
  under the old: two calls, same agent and same relay, differing only in
  `expectedSignerPubkey`.
- Settling one signer's start leaves the other signer's suppression
  intact — per-signer settlement independence.
- The existing same-signer suppression test is the unmodified control,
  as are the cross-community and A→B→A retention tests.

Both new tests were confirmed red with the signer removed from the key,
with the other 18 in the file green. No E2E: unlike b4e87be's retention
fix, the seam here is entirely inside the hook, and every E2E spec runs
under the mock bridge's single constant identity.

Verified: desktop tsc --noEmit, biome check over the touched files,
desktop unit tests (5863 passed, 0 failed), and the mentions Playwright
smoke suite against a fresh pnpm build:e2e bundle (86 passed, 1 failed —
the persona-mention spec flaking under load at the composer-chip step,
before any start assertion; green 3x in isolation). The repository
file-size gate stays green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — Request changes

Reviewed base 4a9de1a3a121285ef475d630b2b5764044c02cde through exact head d7e6e3ec862ea76dc96fbdd2493b63b08015f324.

The changed head correctly repairs signer-aware detached-start coalescing, and the publish-first UX remains truthful and community-scoped. One author-actionable authorization race remains.

[P2] Mention authorization can still go stale before publication

desktop/src/features/messages/ui/useMentionSendFlow.ts:379-390 admits mention pubkeys, then suspends for managed-agent lookup; :430-437 always awaits readiness; and :470-476 awaits huddle IPC. The publish-boundary pass at :527-530 runs only when upload/preview work exists or relaySideEffectsRan is true. A matching access policy (wrote=false) plus no active huddle leaves that flag false despite those suspension points. If authorization is revoked during a held no-write/no-active-huddle leg, the old admitted p tag is still published at :539-546.

The searched publish-boundary regressions cover deferred upload, attach writes, and active-huddle writes (desktop/tests/e2e/mentions.spec.ts:1985-2252), but not revocation during a held no-write/no-active-huddle suspension. The fast-path assertion at :1854-1860 instead requires only one revalidation, preserving the unsafe optimization.

Author action: revalidate whenever any suspension follows admission, or move admission to a genuinely suspension-free publish boundary. Add a causal regression that holds a no-write/no-active-huddle operation, revokes authorization, and proves the outgoing p tag is stripped; mutation-disable the production trigger and require the regression to fail.

Verification owner: author for this fix and causal regression; reviewer for exact-head retrace; CI for pending hosted gates.

Repaired and verified contracts

  • The detached-start key now contains verbatim relay URL, normalized signer, and normalized agent pubkey (useDetachedAgentStart.ts:139-167), matching its Tauri payload (:181-185) and native relay/signer assertions (desktop/src-tauri/src/commands/agents.rs:842-846). Provider deployment reasserts both against the post-lock rebuilt payload (provider_deploy.rs:65-96).
  • Real-hook tests exercise live identity-cache replacement: a new signer on the same relay/agent fires independently, and settlement releases only that signer’s key (useDetachedAgentStart.test.mjs:757-841). The focused detached-start suite passed 20/20 at this head; removing signer from the production key made both new cases fail before restoration.
  • Publication is accepted before queued wakes flush (useMentionSendFlow.ts:539-555). Failed publication restores the draft and does not wake; failed wake truthfully warns that the message was sent but the agent may not respond.
  • Stale failure warnings are fenced by visible relay/signer scope, and failed starts release their map entry for retry (useDetachedAgentStart.ts:139-218).

Exact-head validation

  • Full Desktop JS package: 5,863/5,863 passed.
  • Desktop typecheck: passed.
  • Focused interaction E2E: 9/9 passed, covering publish-before-wake, duplicate suppression, truthful wake failure, failed-publish no-wake/no-false-success, A→B warning isolation, A→B→A dedupe, provider deploy, and managed-agent member/non-member flows.
  • Touched-file Biome check and git diff --check: passed.
  • Live PR head immediately before submission: d7e6e3ec862ea76dc96fbdd2493b63b08015f324; authenticated reviewer jedwards27, author matt2e.

Confidence gaps

The full local smoke-file rerun did not complete after reviewer worktree/webserver teardown; the clean focused rerun passed. Hosted Desktop Core, four smoke shards, relay E2E, and Windows Rust remained in progress at the final snapshot, with no completed required check failing. These are reviewer/tooling or CI-owned gaps, not additional author defects.

Any new head invalidates this verdict.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — REQUEST CHANGES at exact head d7e6e3ec862ea76dc96fbdd2493b63b08015f324 (base 4a9de1a3a121285ef475d630b2b5764044c02cde). This changed-head review supersedes the prior verdict at b4e87be8… and integrates the systems/integration and product/UI/adversarial lanes.

Risk: high — mention authorization, signed publication, detached agent lifecycle, renderer↔Tauri scope, and cross-community failure presentation.

Blocking finding

[P2] Publish-boundary authorization remains stale across post-admission suspensions. desktop/src/features/messages/ui/useMentionSendFlow.ts:379-390 admits mention pubkeys, then awaits managed-agent lookup; :430-437 always awaits readiness, and :470-476 awaits huddle IPC. But :527-530 repeats authorization only for upload/preview work or when relaySideEffectsRan is true. A matching access policy (wrote=false) plus no active huddle leaves that flag false. If authorization is revoked during any held no-write/no-active-huddle operation, the old admitted p tag is still published at :539-546. Existing publish-boundary E2E covers deferred upload and write-producing paths, while the fast-path test explicitly expects one validation; it does not cover causal revocation during these other awaited operations.

Author action: revalidate whenever any suspension follows admission, or move admission to a genuinely suspension-free point immediately before publish. Add a causal held no-write/no-active-huddle revocation regression and mutation-prove its production trigger.

Verification owner: author for fix and causal regression; reviewer for exact-head source/interaction retrace.

Repaired and verified

The prior signer-coalescing defect is repaired. The renderer key now includes verbatim relay plus normalized signer and agent (desktop/src/features/agents/useDetachedAgentStart.ts:139-167), matching the renderer payload and native relay/signer contract. Real-hook tests cover same-relay identity replacement and signer-independent settlement; mutating the production key back to relay+agent made both new signer cases fail before restoration.

The integrated product lane found no additional UI defect. Accepted publication still precedes detached wake; publish failure restores the draft and triggers no wake, while wake failure truthfully preserves the sent message and reports that the agent may not respond. Cross-community warning isolation, retry after failed start, and A→B→A coalescing remained sound in reviewed paths and focused journeys.

Exact-head validation

  • Full Desktop JS package: 5,863/5,863 passed.
  • Desktop typecheck passed; Desktop check exited 0 with only pre-existing diagnostics outside the delta.
  • Focused detached-start suite: 20/20 passed; signer-key causal mutation failed the two intended cases, then restoration passed.
  • Focused product interaction E2E: 9/9 passed, covering publish-before-wake, duplicate suppression, truthful wake failure, failed-publish no-wake/no-false-success, A→B warning isolation, A→B→A dedupe, provider deploy, and member/non-member managed-agent flows.
  • git diff --check b4e87be8...d7e6e3ec passed in clean detached worktrees; both lanes rechecked local/live head equality at d7e6e3ec….

Residual risk / confidence gaps

A broad smoke-file rerun reached 31 passes before reviewer worktree/webserver teardown caused 404/missing-worker failures; the clean focused 9/9 rerun passed. At final refresh, Desktop Core, smoke shards, integration/relay/backend E2E, Unit Tests, and Windows Rust were still running; completed observed gates were green. These are reviewer/tooling or named CI-gate confidence gaps, not additional author work and not the basis for this verdict.

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.

2 participants