mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(onboarding): let Fizz's live callback close the Welcome kickoff
The kickoff scripted a closer and assumed it was the last word. It never could be: the teammate intros `@mention` Fizz, which is the mandatory delegator callback in base_prompt.md, so Fizz is guaranteed to wake and reply after them. The choreography and the base prompt were fighting, and the user got two CTAs. The scripted one was also the one that could be wrong. It raced the intros on an 18s stopwatch (15s wait + 3s beat) — well under a real agent turn, so it reliably announced "Honey and Bumble are taking longer than expected" seconds before both intros landed. Fizz's live reply was better: contextual, and right about the state of the world. So let the guaranteed message be the close, and keep the script for problems only: - buildWelcomeKickoffCloser returns null for a clean kickoff. Failure variants are unchanged and still carry the CTA, where the user needs both the bad news and a way forward. - Enforce the null inside sendWelcomeKickoffCloser rather than at the two call sites, so the delayed-teammate timer can't emit a bare CTA after the intros land and re-classify the kickoff as clean. - TEAMMATE_INTRO_WAIT_MS 15s -> 60s. This only covers a teammate that is alive but silent, where waiting is correct; a crashed teammate is read from agent status and still closes immediately. The closer marker was quietly doubling as the durable "kickoff finished" signal, and a marker requires a message to exist — so "success posts nothing" would have meant "success never latches", refetching the opener subtree forever and restarting the whole Welcome team on every revisit. welcomeKickoffAlreadyFinished now accepts a second piece of relay-side evidence: an intro from every teammate in the opener's thread. The marker still short-circuits first, so the failure paths and the solo opener are untouched. Verified live against Codex on 2026-07-18 with the base prompt as the only variable: the intro loop terminated on its own at four replies. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -13,6 +13,7 @@ import {
|
||||
selectWelcomeKickoffIntroTeammates,
|
||||
waitForWelcomeKickoffBeat,
|
||||
waitForWelcomeTeammatesOnline,
|
||||
welcomeKickoffAlreadyFinished,
|
||||
welcomeTeammateNeedsRestart,
|
||||
} from "./welcomeKickoff.ts";
|
||||
|
||||
@@ -168,8 +169,18 @@ test("kickoff coordinator preserves one task across rerenders and cancels on nav
|
||||
assert.ok(coordinator.begin("welcome"));
|
||||
});
|
||||
|
||||
test("a clean kickoff posts no scripted closer", () => {
|
||||
// The intros @mention the lead, which is the mandatory callback in
|
||||
// base_prompt.md — so the lead is *guaranteed* to wake and reply in-thread.
|
||||
// Scripting a CTA on top of that gave the user two closers, and the scripted
|
||||
// one raced the intros and lost. The lead's live reply is the close now; the
|
||||
// scripted closer is only for reporting a problem.
|
||||
// See docs/welcome-kickoff-silent-failures.md.
|
||||
assert.equal(buildWelcomeKickoffCloser([]), null);
|
||||
assert.equal(buildWelcomeKickoffCloser([], []), null);
|
||||
});
|
||||
|
||||
test("closer degrades coherently for partial and total startup failure", () => {
|
||||
assert.match(buildWelcomeKickoffCloser([]), /What can we help you build/);
|
||||
assert.match(buildWelcomeKickoffCloser(["Honey"]), /Honey is having trouble/);
|
||||
assert.match(
|
||||
buildWelcomeKickoffCloser(["Honey", "Bumble"]),
|
||||
@@ -429,6 +440,81 @@ test("intro replies reach the closer classification without the user opening the
|
||||
);
|
||||
});
|
||||
|
||||
// A clean kickoff posts no closer, so "already finished" can no longer be a
|
||||
// single marker lookup — the marker only exists when something went wrong. If
|
||||
// this regresses, the start effect re-arms on every revisit and restarts the
|
||||
// whole Welcome team each time the channel is opened.
|
||||
test("a clean kickoff reads as finished from the intros, with no closer marker", async () => {
|
||||
const agentSet = { lead: fizz, teammates: [honey, bumble] };
|
||||
const finished = await welcomeKickoffAlreadyFinished(
|
||||
"channel-1",
|
||||
kickoffOpener,
|
||||
agentSet,
|
||||
{
|
||||
closerExists: async () => false,
|
||||
fetchReplies: async () => ({
|
||||
events: [
|
||||
introReply("honey-intro", honey.pubkey, kickoffOpener.id),
|
||||
introReply("bumble-intro", bumble.pubkey, kickoffOpener.id),
|
||||
],
|
||||
nextCursor: null,
|
||||
}),
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(finished, true);
|
||||
});
|
||||
|
||||
test("a kickoff missing one intro is not finished", async () => {
|
||||
const agentSet = { lead: fizz, teammates: [honey, bumble] };
|
||||
const finished = await welcomeKickoffAlreadyFinished(
|
||||
"channel-1",
|
||||
kickoffOpener,
|
||||
agentSet,
|
||||
{
|
||||
closerExists: async () => false,
|
||||
fetchReplies: async () => ({
|
||||
events: [introReply("honey-intro", honey.pubkey, kickoffOpener.id)],
|
||||
nextCursor: null,
|
||||
}),
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(finished, false);
|
||||
});
|
||||
|
||||
test("a closer marker still short-circuits the finished check", async () => {
|
||||
// The failure paths and the solo opener still carry the marker, so that
|
||||
// evidence must keep working without a thread fetch at all.
|
||||
let fetched = false;
|
||||
const finished = await welcomeKickoffAlreadyFinished(
|
||||
"channel-1",
|
||||
kickoffOpener,
|
||||
{ lead: fizz, teammates: [honey, bumble] },
|
||||
{
|
||||
closerExists: async () => true,
|
||||
fetchReplies: async () => {
|
||||
fetched = true;
|
||||
return { events: [], nextCursor: null };
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(finished, true);
|
||||
assert.equal(fetched, false);
|
||||
});
|
||||
|
||||
test("no opener means the kickoff has not finished", async () => {
|
||||
const finished = await welcomeKickoffAlreadyFinished(
|
||||
"channel-1",
|
||||
null,
|
||||
{ lead: fizz, teammates: [honey, bumble] },
|
||||
{ closerExists: async () => false },
|
||||
);
|
||||
|
||||
assert.equal(finished, false);
|
||||
});
|
||||
|
||||
test("merging the opener subtree never double-counts an already-visible reply", () => {
|
||||
const honeyIntro = introReply("honey-intro", honey.pubkey, kickoffOpener.id);
|
||||
// An open thread feeds the same replies in through both sources.
|
||||
|
||||
@@ -24,13 +24,25 @@ import {
|
||||
} from "@/shared/api/tauriManagedAgents";
|
||||
import { hasManagedAgentChannelMessageMarker } from "@/shared/api/tauriManagedAgentMessageMarkers";
|
||||
import { sendManagedAgentChannelMessage } from "@/shared/api/tauriManagedAgentMessages";
|
||||
import { getPresence, listManagedAgents } from "@/shared/api/tauri";
|
||||
import {
|
||||
getPresence,
|
||||
getThreadReplies,
|
||||
listManagedAgents,
|
||||
} from "@/shared/api/tauri";
|
||||
import { getProfile } from "@/shared/api/tauriProfiles";
|
||||
import type { Channel, ManagedAgent, RelayEvent } from "@/shared/api/types";
|
||||
import { normalizePubkey } from "@/shared/lib/pubkey";
|
||||
import { useQueryClient } from "@tanstack/react-query";
|
||||
|
||||
export const WELCOME_KICKOFF_OPENER_MARKER = "buzz-welcome-kickoff.opener.v1";
|
||||
// Despite the name, presence does NOT mean "the kickoff finished": a clean team
|
||||
// kickoff posts no closer at all and is closed by the lead's own callback reply.
|
||||
// This marks only the paths that had something to say — a failed or delayed
|
||||
// teammate, or no team to wait for (the solo opener carries it too; see
|
||||
// `additionalMarkers` below). Ask `welcomeKickoffAlreadyFinished` whether a
|
||||
// kickoff is done: a marker needs a message to exist, so absence proves nothing.
|
||||
// The string stays as-is — renaming it strands the markers already on relays for
|
||||
// kickoffs that failed, and the dedupe below would let them report twice.
|
||||
export const WELCOME_KICKOFF_CLOSER_MARKER = "buzz-welcome-kickoff.closer.v1";
|
||||
export const WELCOME_KICKOFF_PROVIDER_MARKER =
|
||||
"buzz-welcome-kickoff.provider-required.v1";
|
||||
@@ -87,8 +99,19 @@ const kickoffCoordinator = createWelcomeKickoffCoordinator();
|
||||
const closerInFlight = new Set<string>();
|
||||
const TEAMMATE_READY_POLL_MS = 250;
|
||||
const TEAMMATE_READY_WAIT_MS = 60_000;
|
||||
const TEAMMATE_INTRO_WAIT_MS = 15_000;
|
||||
// How long a teammate that is alive but silent gets before the closer reports it
|
||||
// as delayed. This budget has to cover a whole agent turn — wake on the p tag,
|
||||
// fetch context, run inference, shell out to `buzz messages send` — so it is
|
||||
// tens of seconds, not a UI beat. At 15s the closer reliably lost the race and
|
||||
// announced "taking longer than expected" seconds before the intros landed,
|
||||
// contradicting itself in front of a brand new user. A *crashed* teammate does
|
||||
// not wait this out: `failed` is read from agent status and closes immediately,
|
||||
// so this budget only covers the case where patience is the right answer.
|
||||
const TEAMMATE_INTRO_WAIT_MS = 60_000;
|
||||
const CLOSER_BEAT_MS = 3_000;
|
||||
// The intros are the first replies to the opener; this only has to be deep
|
||||
// enough to see them, not to page a real conversation.
|
||||
const INTRO_LOOKUP_LIMIT = 200;
|
||||
const closerAbortControllers = new Map<string, AbortController>();
|
||||
const closerTimeouts = new Map<
|
||||
string,
|
||||
@@ -238,12 +261,24 @@ export async function waitForWelcomeKickoffBeat(options: {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the closer, or `null` when nothing needs to be said.
|
||||
*
|
||||
* The scripted closer exists to report a *problem*: a teammate that crashed or
|
||||
* hasn't spoken yet. A clean kickoff returns `null` and posts nothing, because
|
||||
* the lead already closes the conversation itself — the intros `@mention` the
|
||||
* lead (the mandatory callback in `buzz-acp/src/base_prompt.md`), which wakes it
|
||||
* and it replies in-thread. Scripting a second CTA on top of that gave the user
|
||||
* two closers, and the scripted one — fired on a timer, before the intros were
|
||||
* in — was the one that could be wrong. See
|
||||
* docs/welcome-kickoff-silent-failures.md.
|
||||
*/
|
||||
export function buildWelcomeKickoffCloser(
|
||||
failedNames: readonly string[],
|
||||
delayedNames: readonly string[] = [],
|
||||
) {
|
||||
): string | null {
|
||||
if (failedNames.length === 0 && delayedNames.length === 0) {
|
||||
return WELCOME_KICKOFF_CTA;
|
||||
return null;
|
||||
}
|
||||
if (failedNames.length === 1 && delayedNames.length === 0) {
|
||||
return `${failedNames[0]} is having trouble starting — you can check on them in Agents.\n\n${WELCOME_KICKOFF_CTA}`;
|
||||
@@ -339,6 +374,48 @@ async function markerExists(channelId: string, marker: string) {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Has the kickoff already run to completion in this channel?
|
||||
*
|
||||
* This used to be a single `closerMarker` lookup, which worked only because
|
||||
* every kickoff ended in a scripted message we could tag. A clean kickoff now
|
||||
* posts nothing, so there is no marker to find and the marker check alone would
|
||||
* report "not finished" forever — re-arming the start effect and restarting the
|
||||
* Welcome team on every single revisit.
|
||||
*
|
||||
* So fall back to the other durable, relay-side evidence that it finished: the
|
||||
* opener's thread holds an intro from each teammate. Both signals are
|
||||
* server-side, which is what makes this safe across app restarts and devices —
|
||||
* a local latch would not be.
|
||||
*/
|
||||
export async function welcomeKickoffAlreadyFinished(
|
||||
channelId: string,
|
||||
opener: RelayEvent | null,
|
||||
agentSet: WelcomeAgentSet,
|
||||
options: {
|
||||
closerExists?: (channelId: string) => Promise<boolean>;
|
||||
fetchReplies?: typeof getThreadReplies;
|
||||
} = {},
|
||||
) {
|
||||
const closerExists =
|
||||
options.closerExists ?? ((id: string) => markerExists(id, closerMarker));
|
||||
const fetchReplies = options.fetchReplies ?? getThreadReplies;
|
||||
if (await closerExists(channelId)) return true;
|
||||
if (!opener) return false;
|
||||
const { events } = await fetchReplies(opener.id, channelId, {
|
||||
limit: INTRO_LOOKUP_LIMIT,
|
||||
cursor: null,
|
||||
});
|
||||
const introAuthors = introAuthorsAfterOpener(
|
||||
events,
|
||||
opener,
|
||||
agentSet.teammates,
|
||||
);
|
||||
return agentSet.teammates.every((agent) =>
|
||||
introAuthors.has(normalizePubkey(agent.pubkey)),
|
||||
);
|
||||
}
|
||||
|
||||
export function welcomeTeammateNeedsRestart(
|
||||
agent: ManagedAgent,
|
||||
leadPubkey: string,
|
||||
@@ -453,9 +530,14 @@ async function sendWelcomeKickoffCloser({
|
||||
}: {
|
||||
agentSet: WelcomeAgentSet;
|
||||
channelId: string;
|
||||
content: string;
|
||||
content: string | null;
|
||||
opener: RelayEvent;
|
||||
}) {
|
||||
// A clean kickoff has nothing to report, so it posts nothing and the lead's
|
||||
// own callback reply closes the thread. Enforced here rather than at the call
|
||||
// sites so the delayed-teammate timer can't sneak a bare CTA out after the
|
||||
// intros land and re-classify the kickoff as clean.
|
||||
if (content === null) return;
|
||||
if (await markerExists(channelId, closerMarker)) return;
|
||||
|
||||
await sendManagedAgentChannelMessage({
|
||||
@@ -491,17 +573,24 @@ export function useWelcomeKickoff(
|
||||
() => markerEvent(channelEvents, openerMarker) ?? null,
|
||||
[channelEvents],
|
||||
);
|
||||
// Retire the watch once the closer exists: the kickoff is resolved, so
|
||||
// revisits to Welcome shouldn't keep refetching the subtree forever.
|
||||
const agentSet = React.useMemo(
|
||||
() =>
|
||||
resolveWelcomeAgentSetForRelay(
|
||||
managedAgentsQuery.data ?? [],
|
||||
activeCommunity?.relayUrl,
|
||||
),
|
||||
[activeCommunity?.relayUrl, managedAgentsQuery.data],
|
||||
);
|
||||
// Retire the watch once the kickoff is resolved, so revisits to Welcome don't
|
||||
// keep refetching the subtree forever.
|
||||
//
|
||||
// This has to be a latch rather than a plain derivation. The closer is a
|
||||
// *thread reply* to the opener (see sendWelcomeKickoffCloser), so it never
|
||||
// appears in `channelEvents` unless the user happened to open the thread —
|
||||
// deriving from `channelEvents` meant this never retired at all. Deriving
|
||||
// from `kickoffEvents` instead is self-referential: it gates the query that
|
||||
// feeds it, so retiring would drop the evidence that justified retiring and
|
||||
// (on a cache eviction) flip the query back on. Latching per channel keeps
|
||||
// the decision one-way and stable.
|
||||
// This has to be a latch rather than a plain derivation. The evidence lives in
|
||||
// the opener's *thread*, so it never appears in `channelEvents` unless the user
|
||||
// happened to open the thread — deriving from `channelEvents` meant this never
|
||||
// retired at all. Deriving from `kickoffEvents` instead is self-referential: it
|
||||
// gates the query that feeds it, so retiring would drop the evidence that
|
||||
// justified retiring and (on a cache eviction) flip the query back on. Latching
|
||||
// per channel keeps the decision one-way and stable.
|
||||
const [resolvedChannelId, setResolvedChannelId] = React.useState<
|
||||
string | null
|
||||
>(null);
|
||||
@@ -516,19 +605,33 @@ export function useWelcomeKickoff(
|
||||
);
|
||||
React.useEffect(() => {
|
||||
if (!channelId || kickoffResolved) return;
|
||||
if (markerEvent(kickoffEvents, closerMarker) == null) return;
|
||||
// A closer only exists when something went wrong, so it can't be the only
|
||||
// thing we latch on any more — a clean kickoff would never retire.
|
||||
if (markerEvent(kickoffEvents, closerMarker) != null) {
|
||||
setResolvedChannelId(channelId);
|
||||
return;
|
||||
}
|
||||
if (!openerEvent || !agentSet) return;
|
||||
const { failed, unresolved } = classifyWelcomeKickoffResolution(
|
||||
kickoffEvents,
|
||||
openerEvent,
|
||||
agentSet,
|
||||
);
|
||||
// Every teammate introduced itself and none crashed: the choreography is
|
||||
// done and the lead's callback reply is the close. Nothing further to post.
|
||||
if (failed.length > 0 || unresolved.length > 0) return;
|
||||
// Drop any pending delayed-teammate timer. `sendWelcomeKickoffCloser` would
|
||||
// refuse to post a clean closer anyway, but leaving a timer armed against a
|
||||
// finished kickoff is a trap for the next reader.
|
||||
const pending = closerTimeouts.get(channelId);
|
||||
if (pending) {
|
||||
globalThis.clearTimeout(pending);
|
||||
closerTimeouts.delete(channelId);
|
||||
}
|
||||
setResolvedChannelId(channelId);
|
||||
}, [channelId, kickoffEvents, kickoffResolved]);
|
||||
}, [agentSet, channelId, kickoffEvents, kickoffResolved, openerEvent]);
|
||||
const channelEventsRef = React.useRef(kickoffEvents);
|
||||
channelEventsRef.current = kickoffEvents;
|
||||
const agentSet = React.useMemo(
|
||||
() =>
|
||||
resolveWelcomeAgentSetForRelay(
|
||||
managedAgentsQuery.data ?? [],
|
||||
activeCommunity?.relayUrl,
|
||||
),
|
||||
[activeCommunity?.relayUrl, managedAgentsQuery.data],
|
||||
);
|
||||
const readiness = React.useMemo(
|
||||
() => resolveAgentReadiness(runtimesQuery.data ?? [], globalConfig),
|
||||
[globalConfig, runtimesQuery.data],
|
||||
@@ -562,7 +665,13 @@ export function useWelcomeKickoff(
|
||||
teammates: [welcomeTeam[1], welcomeTeam[2]],
|
||||
};
|
||||
|
||||
if (await markerExists(channelId, closerMarker)) {
|
||||
if (
|
||||
await welcomeKickoffAlreadyFinished(
|
||||
channelId,
|
||||
markerEvent(channelEventsRef.current, openerMarker) ?? null,
|
||||
resolvedAgentSet,
|
||||
)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
if (!readiness.ready) {
|
||||
|
||||
@@ -3,11 +3,14 @@
|
||||
Status: **partly open.** The *perception* gap is handled (see below); the silent
|
||||
paths themselves are not. The [intro loop](#the-intro-loop-a-loud-failure) is
|
||||
mitigated as of 2026-07-18 by three bullets in the base prompt's Callback
|
||||
Mentions section — **not yet confirmed live against Codex.** Prompt is the only
|
||||
lever available: a structural cap was considered and rejected (see that section).
|
||||
Mentions section, and **confirmed live against Codex** — the chain terminated on
|
||||
its own at four replies. Prompt is the only lever available: a structural cap was
|
||||
considered and rejected (see that section). The
|
||||
[double closer](#the-closer-was-racing-a-message-it-could-not-beat) that the same
|
||||
run exposed is fixed as of 2026-07-18.
|
||||
Context: the Welcome-channel kickoff choreography
|
||||
(`desktop/src/features/onboarding/welcomeKickoff.ts`) where Fizz posts an
|
||||
opener, teammates introduce themselves in-thread, and Fizz posts a closer.
|
||||
opener and teammates introduce themselves in-thread.
|
||||
|
||||
Two failure shapes live here: the **silent** ones (nobody speaks) and the
|
||||
**loud** one (nobody stops). They have opposite symptoms and the same root
|
||||
@@ -51,7 +54,8 @@ that runs away.
|
||||
| 1 | Provider fallback ("connect to an AI provider in Settings…") | Readiness check fails before kickoff | Fizz (marker: `provider-required.v1`) |
|
||||
| 2 | Happy-path opener (mentions teammates, asks them to introduce themselves) | Team online | Fizz (marker: `opener.v1`) |
|
||||
| 3 | Degraded opener ("I'm here with Honey and Bumble…") | Fizz online, zero teammates online within 60s | Fizz (opener + closer markers, self-contained) |
|
||||
| 4 | Closer variants (clean / failed / slow teammate wording) | 3s beat after intros resolve, or 15s intro timeout | Fizz (marker: `closer.v1`) |
|
||||
| 4 | Closer variants (failed / slow teammate wording only — a clean kickoff posts nothing) | 3s beat after a teammate is seen crashed, or 60s intro timeout | Fizz (marker: `closer.v1`) |
|
||||
| 4b | The actual close on a clean kickoff | The intros `@mention` Fizz, waking it | Fizz, **LLM-generated** (no marker) |
|
||||
| 5 | Setup-mode nudge ("here's what you still need to configure") | Agent process spawns but its requirements check fails (e.g. missing API key) | The agent process itself (backend, buzz-acp setup-listener mode) |
|
||||
|
||||
## Silent paths (what the user CANNOT be told today)
|
||||
@@ -293,14 +297,18 @@ new work — which needs a judgment call the harness can't currently make.
|
||||
|
||||
### Verification status
|
||||
|
||||
- **Not verified live.** The prompt change is behavioral; only a real Codex
|
||||
session on a fresh Welcome channel can confirm it. Prompt edits have no test
|
||||
that can fail on a literal-minded model, so the desktop/Rust suites say
|
||||
nothing about whether this works.
|
||||
- One run on 2026-07-18 went from 21+ replies to **2** (the two intros, no
|
||||
loop) — but that run had the opener waiver in it too, so it is **confounded
|
||||
and does not count as evidence for the base prompt**. It is the reason the
|
||||
opener was reverted: the next run needs to isolate the variable.
|
||||
- **Verified live against Codex on 2026-07-18, with the base prompt as the only
|
||||
variable** (the opener waiver was reverted first, precisely so this run would
|
||||
isolate it). The chain terminated on its own: intros tagged `@Fizz` (the
|
||||
legitimate `:44` callback), Fizz woke and replied once using plain narrative
|
||||
names and **no tags**, so nobody woke and the chain ended at four replies. The
|
||||
designed exit path is the one that fired.
|
||||
- An earlier run went from 21+ replies to 2, but it had the opener waiver in it
|
||||
and is **confounded** — it does not count as evidence for the base prompt. It
|
||||
is the reason the opener was reverted.
|
||||
- Prompt edits have no test that can fail on a literal-minded model, so the
|
||||
desktop/Rust suites say nothing about whether this holds. One live run is
|
||||
encouraging, not proof: n=1 on a sampled model.
|
||||
- Unit-tested only: the solo opener mentions no agent at all (pins the
|
||||
degraded path, which cannot loop by construction).
|
||||
|
||||
@@ -326,6 +334,95 @@ new work — which needs a judgment call the harness can't currently make.
|
||||
off", which reads terminal — but the boundary is a model judgment, and reading
|
||||
it too broadly converts this loud failure into the silent kind.
|
||||
|
||||
## The closer was racing a message it could not beat
|
||||
|
||||
Found in the same 2026-07-18 Codex run that cleared the intro loop. The user saw
|
||||
Fizz say *"Honey and Bumble are taking longer than expected"* — and then both
|
||||
intros landed one minute later, followed by Fizz closing again, properly. Two
|
||||
CTAs, and the wrong one came first.
|
||||
|
||||
Neither symptom was a failure state. They were **one design conflict**:
|
||||
|
||||
1. **The closer raced the intros on a 15s stopwatch.** `TEAMMATE_INTRO_WAIT_MS`
|
||||
was `15_000`, plus a `CLOSER_BEAT_MS` 3s beat — about 18s for a teammate to
|
||||
have an intro *published*. That has to cover a whole agent turn: wake on the p
|
||||
tag, fetch context, run inference, shell out to `buzz messages send`. Real
|
||||
turns run tens of seconds, so the closer reliably lost and announced a delay
|
||||
that wasn't real. PR #2066 tightened this beat, which made it more likely, not
|
||||
less.
|
||||
2. **The scripted closer could never be the last word.** The choreography assumed
|
||||
it was. But the intros `@mention` Fizz — that is the *mandatory* callback in
|
||||
`base_prompt.md:44`, since Fizz delegated the intros — so Fizz is **guaranteed**
|
||||
to wake and reply after them. The choreography and the base prompt were
|
||||
fighting: one scripted a final message, the other guaranteed a message after
|
||||
it.
|
||||
|
||||
Worth noting which one was better: Fizz's live reply ("bring us a project, bug,
|
||||
question, or half-formed idea and I'll route it or start building") beat the
|
||||
scripted CTA. It was contextual, and it was right about the state of the world.
|
||||
|
||||
### The fix (landed 2026-07-18)
|
||||
|
||||
**Let the guaranteed message be the close, and keep the script for problems only.**
|
||||
|
||||
1. `buildWelcomeKickoffCloser` returns **`null`** for a clean kickoff — the
|
||||
success state posts nothing. The failure variants (crashed teammate, slow
|
||||
teammate) are unchanged and still carry the CTA, because there the user needs
|
||||
both the bad news and a way forward.
|
||||
2. The `null` is enforced inside `sendWelcomeKickoffCloser`, not at the call
|
||||
sites. There are two callers, and the delayed-teammate timer can fire *after*
|
||||
the intros land and re-classify the kickoff as clean — the choke point stops
|
||||
that stray timer from emitting a bare CTA.
|
||||
3. `TEAMMATE_INTRO_WAIT_MS` `15_000` → `60_000`, matching the presence wait. This
|
||||
budget now only covers a teammate that is *alive but silent*, where patience is
|
||||
the correct answer — a **crashed** teammate is read from agent status
|
||||
(`failed`) and closes immediately, so real breakage is still reported fast.
|
||||
|
||||
### The latch this broke, and why it needed a second signal
|
||||
|
||||
The closer marker was doing quiet double duty: it was also the durable *"kickoff
|
||||
already finished"* signal, in two places — retiring the opener-thread watch, and
|
||||
the guard in the start effect that stops the choreography re-arming on revisit.
|
||||
A marker only exists if a **message** exists (`markerExists` resolves it by
|
||||
querying the relay for a tagged message), so "success posts nothing" silently
|
||||
means "success never latches". Left alone that would have:
|
||||
|
||||
- refetched the opener subtree forever on every Welcome revisit — the exact bug
|
||||
PR #2066 fixed; and worse,
|
||||
- **restarted the whole Welcome team every time the channel was opened**, because
|
||||
the start effect's guard would never trip.
|
||||
|
||||
So completion needs a second piece of evidence, and it has to be **relay-side** —
|
||||
a local latch would re-run on another device or after a reinstall.
|
||||
`welcomeKickoffAlreadyFinished` now reads: *closer marker exists* **OR** *the
|
||||
opener's thread holds an intro from every teammate*. Both are durable server-side
|
||||
facts. The marker still short-circuits first, so the failure paths and the solo
|
||||
opener (which carries both markers on one message) are untouched and cost no
|
||||
extra fetch.
|
||||
|
||||
The in-memory latch stays a latch for the reason the original comment gives: the
|
||||
evidence lives in the subtree that the latch itself gates, so deriving it fresh
|
||||
each render is self-referential.
|
||||
|
||||
### Residual risk
|
||||
|
||||
- **The CTA is now model-generated on the happy path.** If Fizz wakes and says
|
||||
something that doesn't invite the user in, the kickoff ends softer than the
|
||||
script did. The mitigation is `base_prompt.md:44` making the wake itself
|
||||
guaranteed — but *what Fizz says* is not. This is the deliberate bet of the
|
||||
change, and the thing to watch across runs.
|
||||
- **If Fizz doesn't reply at all, nothing closes.** The intros still landed, so
|
||||
the channel is a usable welcome rather than an empty one — but there is no CTA
|
||||
and no scripted backstop. A watchdog (wait N seconds for the lead's callback,
|
||||
then post the scripted CTA) was considered and deliberately left out: it re-adds
|
||||
the timer race this change removed. Revisit only if a run shows the lead going
|
||||
quiet.
|
||||
- Unit tests pin the contract (`buildWelcomeKickoffCloser([])` is `null`; the
|
||||
finished-check reads the intros with no marker; a marker short-circuits without
|
||||
a fetch; a missing intro is not finished). What they **cannot** pin is whether a
|
||||
real Fizz reliably produces a good close — same limitation as the intro-loop
|
||||
fix.
|
||||
|
||||
## Constraints for the silent-path fix
|
||||
|
||||
- **Fizz cannot be the messenger** for these paths: she is the thing that
|
||||
@@ -391,3 +488,28 @@ new work — which needs a judgment call the harness can't currently make.
|
||||
- The two failure classes interact: silent-path work adds messages, and every
|
||||
new message is a potential wake. Any fallback that `@mentions` an agent to
|
||||
recover from a failure can itself seed a loop.
|
||||
- **Open thread panes can go stale while the reply count keeps climbing** — a
|
||||
general bug, not a kickoff bug, but it shows up here because the intros are
|
||||
thread replies. Observed repeatedly, including in the 2026-07-18 run. The cause
|
||||
of the *asymmetry* is confirmed: the count and the list read from different
|
||||
sources. The count is the relay's server-computed recount pushed as kind 39005
|
||||
(`hooks.ts`, `channelWindowStore.ts` `mergeLiveThreadSummary`) — authoritative
|
||||
and independent of the reply event itself. The list is a client-maintained cache
|
||||
(`["thread-replies", channelId, rootId]`) appended to by parsing live events.
|
||||
One is robust, the other is fragile, so they can disagree.
|
||||
|
||||
The specific trigger is **not** confirmed. Leading hypothesis: a refetch
|
||||
overwrite race in `useThreadReplies.ts` — the query is `staleTime: 0`, and on
|
||||
refetch it replaces the cache with the server's answer while preserving only
|
||||
replies that arrived *during* the fetch (`receivedInFlight` is diffed against
|
||||
`idsAtStart`). A live reply that landed just *before* a refetch, and that the
|
||||
server's answer doesn't yet include, is dropped. Welcome refetches unusually
|
||||
often because there are two observers on that cache entry (the pane's, plus
|
||||
`welcomeKickoff.ts`'s opener-subtree watch). Fits every symptom, including why
|
||||
close+reopen fixes it (the later fetch catches up). Ruled out for this case:
|
||||
malformed `e` tags (the CLI/SDK emit proper NIP-10 markers) and root-key
|
||||
divergence (the opener *is* the NIP-10 root).
|
||||
|
||||
Note the live-append branch that feeds the pane has **no test coverage at all**,
|
||||
which is why this can regress quietly. Needs a repro test before a fix — don't
|
||||
fix on the hypothesis.
|
||||
|
||||
Reference in New Issue
Block a user