Add customizable welcome message builder - #6947
Conversation
Signed-off-by: kenny lopez <klopez4212@gmail.com>
Signed-off-by: kenny lopez <klopez4212@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec7d9cd4cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function saveMessage() { | ||
| setSavedMessage(message); | ||
| setBuilderOpen(false); | ||
| toast.success("Welcome message saved"); |
There was a problem hiding this comment.
saveMessage only copies the draft into component state and displays a success toast; no Tauri command, relay event, or storage write occurs, and no join workflow reads this state. Consequently, leaving the settings view or restarting the app loses the “saved” message, and new members never receive or see it. Persist and hydrate this as community-scoped data, preferably through the repository’s Nostr event path.
AGENTS.md reference: AGENTS.md:L158-L172
Useful? React with 👍 / 👎.
| if let Some(home) = default_agent_workdir() { | ||
| command.current_dir(home); |
There was a problem hiding this comment.
When the configured runtime is a tool-capable coding agent such as Goose, Codex, or Claude, this helper runs it from ~/.buzz (or the user’s home directory), while AcpClient::handle_permission_request automatically selects every allow_once permission. The supposedly text-only writing request can therefore execute shell or file-edit tools against a directory containing Buzz configuration and credentials. Run this helper in an isolated temporary directory and reject tool permissions or explicitly apply a read-only/plan mode.
Useful? React with 👍 / 👎.
Signed-off-by: kenny lopez <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe090044d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| #[derive(Debug, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct WelcomeMessageDraft { |
There was a problem hiding this comment.
WelcomeMessageDraft is a newly exported Rust type but has no rustdoc, violating the repository requirement that new public APIs have doc comments. Add a concise description of the serialized draft contract before exporting it.
AGENTS.md reference: AGENTS.md:L147-L150
Useful? React with 👍 / 👎.
| for (key, value) in &descriptor.env { | ||
| command.env(key, value); | ||
| } |
There was a problem hiding this comment.
When the preferred runtime is Claude or Codex and the configured global model differs from the CLI default, forwarding only descriptor.env loses that selection: those runtimes have no model_env_var, while the normal managed-agent spawn separately applies ANTHROPIC_MODEL or BUZZ_ACP_MODEL, and run_one_shot_prompt does not select a model after creating the session. Writing help therefore silently runs the runtime's default model, potentially changing behavior and cost; propagate and apply the resolved model through the one-shot path.
Useful? React with 👍 / 👎.
| match update_type { | ||
| "agent_message_chunk" => { | ||
| if let Some(text) = update["content"]["text"].as_str() { | ||
| self.agent_message.push_str(text); |
There was a problem hiding this comment.
For every ordinary relay-driven agent turn, this now appends all agent_message_chunk text even though only the one-shot helper consumes the buffer. The existing LinesCodec limit only bounds each individual line, so a verbose or rogue adapter can stream bounded chunks throughout the two-hour turn and make every agent client retain an arbitrarily large string, defeating the read-path OOM protection. Enable capture only for helper clients or impose an aggregate response cap.
Useful? React with 👍 / 👎.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — REQUEST CHANGES
Reviewed: 0ccf934b88f610f5f235862ecd51e2dcbae2cb74..fe090044d84f853315fdba229a4578608ba69ef9 (live head rechecked immediately before submission)
Risk: high — this adds a community-admin setting whose claimed persistence and member-join delivery cross renderer, native, relay, authorization, and lifecycle boundaries.
Blocking — Save neither persists nor delivers the welcome message
desktop/src/features/community-members/ui/WelcomeChannelSettings.tsx:38-40 holds the only saved value in component-local React state. saveMessage() at :74-78 only copies the draft into that state, closes the dialog, and displays “Welcome message saved.” The new native command generates a draft, but the changed production paths contain no durable save/load contract or join-path consumer.
Consequently, a remount/restart discards the setting, another admin/device cannot observe it, and a joining member never receives or renders it. The focused Playwright flow only closes and reopens the dialog within the same mounted component, so it protects the illusion rather than the stated contract: “Set the message new members see when they join.”
Author action: either:
- implement community-scoped, authorized durable save/load and authoritative join-path consumption; report success only after durable completion; expose loading/save/delivery failures; and add regressions proving persistence across remount/reload plus delivery/rendering for a joining member, including tenant/auth and retry/duplicate boundaries; or
- if this is intentionally builder-only, remove the Save/live-delivery claims and success toast, then retitle/rescope the feature truthfully.
Verification owner: author for the blocking implementation and contract-shaped regressions; reviewer for revalidation of the admin/new-member journey and visible success/error states.
Exact-head validation
- PASS —
cargo test -p buzz-acp: 823 unit + 9 lifecycle integration tests, 0 failures. - PASS —
pnpm --dir desktop test: 5,706 tests, 0 failures. - PASS —
just desktop-tauri-test: full Tauri workspace suite; one test ignored by declaration. - PASS — desktop
tsc --noEmit. - PASS —
invites-settings-screenshots.spec.ts: 6/6; inspected builder, formatting/preview, context-menu, assisted-writing, and image-preview output with no separate visual blocker. - PASS — exact-head GitHub functional matrix: Desktop/Core/smoke/integration/relay, Rust, Security, cross-compile, DCO, macOS build, and release candidate. The lone red
Mark Previous Review Stalejob is review-workflow bookkeeping, not a PR-caused product gate. - PASS — clean detached worktree and
git diff --checkat the reviewed SHA.
Confidence gaps — not additional blockers
- No packaged native journey was independently exercised. Reviewer/tooling owns that verification; it is not additional author rework.
- The focused Rust attempts in one validation lane timed out while downloading/compiling; the completed affected-package suites and exact-head CI passed.
- Pasted rich content is serialized as HTML and reconstructed with
template.innerHTML. No unsafe retained element/attribute was reproduced through the actual contenteditable path, so this remains residual security risk rather than a finding; a targeted sanitization test would improve confidence. - Provider-specific ACP one-shot output/chunk behavior was not exercised against every supported adapter.
The editor is polished. Unfortunately, it is attached to no system of record and no join lifecycle. The primary success state is therefore materially false.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
REQUEST CHANGES on fe090044d84f853315fdba229a4578608ba69ef9 against 0ccf934b88f610f5f235862ecd51e2dcbae2cb74. I reviewed GitHub metadata, diff, and exact-head source only; I did not execute PR code.
P1 — Save neither persists nor delivers the advertised welcome message
WelcomeChannelSettings.tsx:38-40 keeps the only saved value in component-local React state, and saveMessage() at :74-78 only copies the draft into that state, closes the dialog, and reports “Welcome message saved.” Across the changed production paths, generate_welcome_message only returns an assisted draft; there is no community-scoped durable write/read or join-time consumer.
The setting therefore disappears on remount/restart, cannot propagate to another admin/device, and cannot produce the message promised by “Set the message new members see when they join.” The Playwright flow protects the local-state illusion by checking the dialog closes without reloading or joining a member.
Please either implement an authorized community-scoped save/load and authoritative join-delivery/rendering path, with causal coverage for remount/reload, community isolation, and an actual member join, or rescope the UI so it no longer claims this is saved or delivered.
P1 security — The writing-only helper can authorize coding tools in Buzz’s sensitive working directory
welcome_message.rs:80-103 launches the globally configured ACP coding runtime from default_agent_workdir() (the user home/~/.buzz) and forwards its resolved environment and provider credentials. The shared ACP client handles every session/request_permission by selecting allow_once (acp.rs:1936-1980), and run_one_shot_prompt creates a normal session with no tool-denial/read-only policy (lib.rs:5092-5108). Thus a feature presented as text generation can execute shell or file tools against Buzz configuration and credentials if the model requests them.
Run this narrow writing task in an isolated, empty working directory with an explicit no-tools policy (not merely a prompt instruction), and add a regression proving a requested tool permission is rejected. Do not expose the user’s full coding-agent environment unless the writer strictly needs each value.
P2 — Assistant-output capture is unbounded for every normal ACP turn
acp.rs:1766-1769 now appends every agent_message_chunk to a String on the shared client, although only the one-shot helper consumes it. The 10 MB codec limit applies per line, not to the aggregate; an ordinary long-running or rogue adapter can keep sending valid bounded chunks until the turn’s hard deadline and grow process memory without a cap. Enable capture only for one-shot clients or enforce a small aggregate limit with a defined overflow error and test.
CI is broadly green; that does not exercise the absent persistence/delivery contract or make the tool boundary safe.
Signed-off-by: kenny lopez <klopez4212@gmail.com>
…ssage-builder Signed-off-by: kenny lopez <klopez4212@gmail.com>
Signed-off-by: kenny lopez <klopez4212@gmail.com>
🔐 Codex Security Review
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8ae0f486a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (key, value) in &descriptor.env { | ||
| command.env(key, value); | ||
| } |
There was a problem hiding this comment.
When the selected harness definition or global agent environment sets the supported BUZZ_ACP_SYSTEM_PROMPT knob, this later environment loop overwrites WELCOME_WRITER_SYSTEM_PROMPT on the child command. Writing help then receives the ordinary agent prompt instead of the fixed plain-JSON/no-tools prompt, which can make otherwise valid configurations return tool activity or unparsable output. Apply the writer-specific value after descriptor.env, or exclude this key from the inherited environment for this helper.
Useful? React with 👍 / 👎.
| let system_prompt_transport = if is_goose || (protocol_version < 2 && !is_claude_agent) { | ||
| None |
There was a problem hiding this comment.
When Goose is the configured runtime, this branch deliberately omits the system prompt from session/new, but the one-shot path never follows up with session_set_goose_system_prompt as the normal session setup in pool.rs does. Consequently Goose never receives WELCOME_WRITER_SYSTEM_PROMPT, so the writing helper loses its intended plain-text/no-tools instruction and may produce output that fails draft parsing. Invoke Goose's custom system-prompt request after creating the session, with the same fallback behavior as the pooled path.
Useful? React with 👍 / 👎.
Signed-off-by: kenny lopez <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b65fdc9245
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| || !valid_type | ||
| || insert.title.trim().is_empty() | ||
| || insert.title.len() > 120 | ||
| || insert.url.len() > 2_048 |
There was a problem hiding this comment.
When writing help is asked to mention a channel such as #introductions, the model receives neither the community’s channel catalog nor the UUID-based buzz://channel/<uuid> contract, and this validation accepts any destination up to 2,048 characters. The E2E mock consequently returns buzz://channel/introductions, which parseChannelLink rejects as invalid-channel-uuid; this draft can still be saved without warning, leaving the welcome message with a nonfunctional channel chip. Pass resolvable channel data into the generation flow or require the admin to select a real channel before accepting the draft.
Useful? React with 👍 / 👎.
Signed-off-by: kenny lopez <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 846eed7271
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (channel) => | ||
| channel.channelType !== "dm" && channel.archivedAt === null, |
There was a problem hiding this comment.
When an admin belongs to a private channel, this filter includes it in the picker even though the welcome message is intended for every newly joined community member. Relay membership does not grant private-channel membership, and isChannelReferenceOpenable rejects private references for non-members, so selecting one produces a channel chip that most new members cannot open; restrict choices to open channels or couple the welcome flow to explicit channel admission.
AGENTS.md reference: AGENTS.md:L11-L18
Useful? React with 👍 / 👎.
Signed-off-by: Clay Delk <clay.delk@gmail.com>
…ssage-builder * origin/main: refactor(db): extract domain stores from database runtime (#6987) feat(desktop): add team sharing to community catalog (#3995) Refresh mobile utility surfaces and theme picker (#6944) fix(desktop): complete project empty and context states (#6980) Fix mobile jump-to-latest flicker (#6807) refactor(relay): NIP-98 admin auth with Operator/Moderator roles and NIP-11 discovery (#3777) refactor(db): split channel membership store (#6782) feat(auth): add NIP-FI canonical assertion verifier and contracts (#6776) Signed-off-by: Clay Delk <clay.delk@gmail.com>
4 participants
Summary
Testing