Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(desktop): converge Welcome teammate access remediation
`provisionWelcomeTeam` decided whether a teammate needed an access write
with `welcomeTeammateHasExpectedAccess`, which requires `owner-only` and
an empty allowlist under the owner-only-access build policy, but then
wrote `allowlist:[lead]` unconditionally. An upgraded install with
pre-existing allowlisted teammates therefore rewrote state the predicate
rejects on every provisioning pass, and `welcomeTeammateNeedsRestart`
kept restarting teammates that were already running.

Extract `welcomeTeammateAccessUpdate`, which returns the write that
satisfies the predicate for the current build (or null when the teammate
is already correct), and use it for the decision and the write so the two
cannot drift again.

Co-authored-by: Tom Brow <tomb@block.xyz>
Signed-off-by: Tom Brow <tomb@block.xyz>
  • Loading branch information
npub12uu53ml9upy7ww9apmtv6vm0u8xlcldx7znsjvwgsr7uvy5g0kssw943ca and brow committed Aug 5, 2026
commit dfad534440e87578e59a7c44f24cf81f31883c43
53 changes: 53 additions & 0 deletions desktop/src/features/onboarding/welcomeGuide.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
pickWelcomeGuideAgentForRelay,
pickWelcomeTeamStarterAgentForRelay,
welcomeStarterRuntimeUpdate,
welcomeTeammateAccessUpdate,
welcomeTeammateHasExpectedAccess,
WELCOME_GUIDE_AGENT_NAME,
WELCOME_GUIDE_PERSONA_ID,
Expand Down Expand Up @@ -390,6 +391,58 @@ test("owner-only-access policy accepts local Welcome teammates", () => {
assert.equal(welcomeTeammateHasExpectedAccess(teammate, PUB_B, false), false);
});

test("access remediation converges for an upgraded owner-only install", () => {
// Pre-existing installs allowlisted the lead. An owner-only build must move
// them to owner-only, and the write it makes must satisfy the predicate, so
// the next provisioning pass makes no further write.
const allowlisted = makeAgent({
respondTo: "allowlist",
respondToAllowlist: [PUB_B],
});
const update = welcomeTeammateAccessUpdate(allowlisted, PUB_B, true);
assert.deepEqual(update, {
pubkey: PUB_A,
respondTo: "owner-only",
respondToAllowlist: [],
});
const remediated = makeAgent({
respondTo: update.respondTo,
respondToAllowlist: update.respondToAllowlist,
});
assert.equal(welcomeTeammateHasExpectedAccess(remediated, PUB_B, true), true);
assert.equal(welcomeTeammateAccessUpdate(remediated, PUB_B, true), null);
});

test("access remediation allowlists the lead when the build is not owner-only", () => {
const ownerOnly = makeAgent({
respondTo: "owner-only",
respondToAllowlist: [],
});
const update = welcomeTeammateAccessUpdate(ownerOnly, PUB_B, false);
assert.deepEqual(update, {
pubkey: PUB_A,
respondTo: "allowlist",
respondToAllowlist: [PUB_B],
});
const remediated = makeAgent({
respondTo: update.respondTo,
respondToAllowlist: update.respondToAllowlist,
});
assert.equal(
welcomeTeammateHasExpectedAccess(remediated, PUB_B, false),
true,
);
assert.equal(welcomeTeammateAccessUpdate(remediated, PUB_B, false), null);
});

test("access remediation skips a teammate that already allows the lead", () => {
const allowlisted = makeAgent({
respondTo: "allowlist",
respondToAllowlist: [PUB_B, PUB_C],
});
assert.equal(welcomeTeammateAccessUpdate(allowlisted, PUB_B, false), null);
});

test("owner-only-access policy accepts provider Welcome teammates", () => {
const teammate = makeAgent({
backend: { type: "provider", id: "remote", config: {} },
Expand Down
42 changes: 35 additions & 7 deletions desktop/src/features/onboarding/welcomeGuide.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {
AgentPersona,
CreateManagedAgentInput,
ManagedAgent,
UpdateManagedAgentInput,
} from "@/shared/api/types";
import { normalizePubkey } from "@/shared/lib/pubkey";

Expand Down Expand Up @@ -282,6 +283,37 @@ export function welcomeTeammateHasExpectedAccess(
);
}

/**
* The access write that moves a Welcome teammate to the state this build
* expects, or null when it is already there. The remediation target must track
* {@link welcomeTeammateHasExpectedAccess}: writing `allowlist:[lead]` in an
* owner-only build would fail the predicate again on the next provisioning
* pass, so an upgraded install with pre-existing allowlisted teammates would
* rewrite the same rejected state forever and keep restarting them.
*/
export function welcomeTeammateAccessUpdate(
teammate: ManagedAgent,
leadPubkey: string,
agentAccessOwnerOnly: boolean,
): UpdateManagedAgentInput | null {
if (
welcomeTeammateHasExpectedAccess(teammate, leadPubkey, agentAccessOwnerOnly)
) {
return null;
}
return agentAccessOwnerOnly
? {
pubkey: teammate.pubkey,
respondTo: "owner-only",
respondToAllowlist: [],
}
: {
pubkey: teammate.pubkey,
respondTo: "allowlist",
respondToAllowlist: [leadPubkey],
};
}

/**
* Ensure the complete built-in Welcome Team is ready for kickoff.
* The team itself is Rust-seeded; this only activates personas, creates any
Expand Down Expand Up @@ -346,17 +378,13 @@ async function provisionWelcomeTeam(
const leadPubkey = lead.pubkey;
for (const index of [1, 2] as const) {
const teammate = welcomeAgents[index];
const alreadyAllowsLead = welcomeTeammateHasExpectedAccess(
const accessUpdate = welcomeTeammateAccessUpdate(
teammate,
leadPubkey,
agentAccessOwnerOnly,
);
if (!alreadyAllowsLead) {
const updated = await updateManagedAgent({
pubkey: teammate.pubkey,
respondTo: "allowlist",
respondToAllowlist: [leadPubkey],
});
if (accessUpdate) {
const updated = await updateManagedAgent(accessUpdate);
welcomeAgents[index] = updated.agent;
}
}
Expand Down