diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 5857ce5a1e..cf35ca34b0 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -5874,6 +5874,54 @@ mod tests { assert!(json_to_context_message(&obj).is_none()); } + #[test] + fn test_json_to_context_message_projects_only_strict_board_references() { + let workstream_id = "123e4567-e89b-12d3-a456-426614174000"; + let valid = json!({ + "kind": "workstream", + "identity": workstream_id, + "snapshot": { "label": "Checkout recovery" }, + "placement": { + "workstreamId": workstream_id, + "workstreamLabel": "checkout-recovery", + "sectionId": "workstream", + "sectionLabel": "Workstream" + } + }); + let mut unknown = valid.clone(); + unknown["snapshot"]["extra"] = json!(true); + let mut controlled = valid.clone(); + controlled["snapshot"]["label"] = json!("bad\u{0085}label"); + let mut forged = valid.clone(); + forged["identity"] = json!("123e4567-e89b-12d3-a456-426614174001"); + let tag = |value: serde_json::Value| { + json!([ + buzz_sdk::BOARD_REFERENCE_TAG, + buzz_sdk::BOARD_REFERENCE_VERSION, + serde_json::to_string(&value).unwrap() + ]) + }; + let obj = json!({ + "pubkey": "abc", + "content": "hello", + "created_at": 1710518400, + "tags": [ + ["h", workstream_id], + tag(unknown), + tag(controlled), + tag(forged), + tag(valid.clone()) + ] + }); + + let msg = json_to_context_message(&obj).expect("message survives bad references"); + assert_eq!(msg.board_references.len(), 1); + assert_eq!( + serde_json::to_value(&msg.board_references[0]).unwrap(), + valid + ); + } + #[test] fn test_collect_prompt_pubkeys_includes_authors_mentions_and_context() { let keys = Keys::generate(); diff --git a/crates/buzz-sdk/src/board_references.rs b/crates/buzz-sdk/src/board_references.rs index 68f6aff4b8..43fbbb4ba9 100644 --- a/crates/buzz-sdk/src/board_references.rs +++ b/crates/buzz-sdk/src/board_references.rs @@ -106,13 +106,16 @@ pub enum BoardReference { } impl BoardReference { - fn dedup_key(&self) -> (&'static str, String) { + fn dedup_key(&self) -> (String, &'static str, String) { + let workstream_id = self.placement().workstream_id.clone(); match self { - Self::Workstream { identity, .. } => ("workstream", identity.clone()), - Self::PullRequest { identity, .. } => ("pull-request", identity.clone()), - Self::Agent { identity, .. } => ("agent", identity.clone()), - Self::AgentGroup { identity, .. } => ("agent-group", identity.clone()), - Self::ThreadBlocker { identity, .. } => ("thread-blocker", identity.clone()), + Self::Workstream { identity, .. } => (workstream_id, "workstream", identity.clone()), + Self::PullRequest { identity, .. } => (workstream_id, "pull-request", identity.clone()), + Self::Agent { identity, .. } => (workstream_id, "agent", identity.clone()), + Self::AgentGroup { identity, .. } => (workstream_id, "agent-group", identity.clone()), + Self::ThreadBlocker { identity, .. } => { + (workstream_id, "thread-blocker", identity.clone()) + } } } @@ -157,6 +160,7 @@ impl BoardReference { && valid_label(&snapshot.label) && snapshot.detail.as_deref().is_none_or(valid_label) && match self { + Self::Workstream { identity, .. } => identity == &placement.workstream_id, Self::Agent { identity, .. } => valid_hex64(identity), Self::ThreadBlocker { destination, .. } => { valid_uuid(&destination.channel_id) @@ -263,15 +267,15 @@ mod tests { use super::*; use nostr::Tags; - fn reference(identity: &str) -> BoardReference { + fn reference(workstream_id: &str) -> BoardReference { BoardReference::Workstream { - identity: identity.into(), + identity: workstream_id.into(), snapshot: BoardSnapshot { label: "Checkout recovery".into(), detail: Some("Active".into()), }, placement: BoardPlacement { - workstream_id: "123e4567-e89b-12d3-a456-426614174000".into(), + workstream_id: workstream_id.into(), workstream_label: "checkout-recovery".into(), section_id: "workstream".into(), section_label: "Workstream".into(), @@ -281,29 +285,102 @@ mod tests { #[test] fn round_trip_preserves_order() { - let tags = build_board_reference_tags(&[reference("one"), reference("two")]).unwrap(); + let tags = build_board_reference_tags(&[ + reference("123e4567-e89b-12d3-a456-426614174000"), + reference("123e4567-e89b-12d3-a456-426614174001"), + ]) + .unwrap(); let parsed = parse_board_references(&Tags::from_list(tags)); - assert_eq!(parsed, vec![reference("one"), reference("two")]); + assert_eq!( + parsed, + vec![ + reference("123e4567-e89b-12d3-a456-426614174000"), + reference("123e4567-e89b-12d3-a456-426614174001") + ] + ); } #[test] fn malformed_unknown_and_duplicate_entries_fail_closed() { - let mut tags = build_board_reference_tags(&[reference("one")]).unwrap(); + let mut tags = + build_board_reference_tags(&[reference("123e4567-e89b-12d3-a456-426614174000")]) + .unwrap(); tags.push(Tag::parse([BOARD_REFERENCE_TAG, "99", "{}"]).unwrap()); tags.push(Tag::parse([BOARD_REFERENCE_TAG, BOARD_REFERENCE_VERSION, "not-json"]).unwrap()); - tags.extend(build_board_reference_tags(&[reference("one"), reference("two")]).unwrap()); + tags.extend( + build_board_reference_tags(&[ + reference("123e4567-e89b-12d3-a456-426614174000"), + reference("123e4567-e89b-12d3-a456-426614174001"), + ]) + .unwrap(), + ); assert_eq!( parse_board_references(&Tags::from_list(tags)), - vec![reference("one"), reference("two")] + vec![ + reference("123e4567-e89b-12d3-a456-426614174000"), + reference("123e4567-e89b-12d3-a456-426614174001") + ] ); } #[test] - fn cross_workstream_references_fail_closed() { - let allowed = reference("allowed"); - let mut other = reference("other"); - if let BoardReference::Workstream { placement, .. } = &mut other { - placement.workstream_id = "123e4567-e89b-12d3-a456-426614174001".into(); + fn strict_unknown_controls_and_forged_workstream_binding_fail_closed() { + let valid = reference("123e4567-e89b-12d3-a456-426614174000"); + let payload = serde_json::to_value(&valid).unwrap(); + let mut invalid = Vec::new(); + for path in ["top", "snapshot", "placement"] { + let mut value = payload.clone(); + let target = match path { + "top" => value.as_object_mut().unwrap(), + "snapshot" => value.get_mut("snapshot").unwrap().as_object_mut().unwrap(), + _ => value.get_mut("placement").unwrap().as_object_mut().unwrap(), + }; + target.insert("extra".into(), serde_json::json!(true)); + invalid.push(value); } + let mut controlled = payload.clone(); + controlled["snapshot"]["label"] = serde_json::json!("bad\u{0085}label"); + invalid.push(controlled); + let mut forged = payload; + forged["identity"] = serde_json::json!("123e4567-e89b-12d3-a456-426614174001"); + invalid.push(forged); + + let mut tags = invalid + .into_iter() + .map(|value| { + Tag::parse([ + BOARD_REFERENCE_TAG, + BOARD_REFERENCE_VERSION, + serde_json::to_string(&value).unwrap().as_str(), + ]) + .unwrap() + }) + .collect::>(); + tags.extend(build_board_reference_tags(&[valid.clone()]).unwrap()); + assert_eq!(parse_board_references(&Tags::from_list(tags)), vec![valid]); + } + + #[test] + fn dedup_key_is_scoped_to_workstream_placement() { + let first = reference("123e4567-e89b-12d3-a456-426614174000"); + let mut second = first.clone(); + if let BoardReference::Workstream { + identity, + placement, + .. + } = &mut second + { + *identity = "123e4567-e89b-12d3-a456-426614174001".into(); + placement.workstream_id = identity.clone(); + } + let tags = + Tags::from_list(build_board_reference_tags(&[first.clone(), second.clone()]).unwrap()); + assert_eq!(parse_board_references(&tags), vec![first, second]); + } + + #[test] + fn cross_workstream_references_fail_closed() { + let allowed = reference("123e4567-e89b-12d3-a456-426614174000"); + let other = reference("123e4567-e89b-12d3-a456-426614174001"); let tags = Tags::from_list(build_board_reference_tags(&[allowed.clone(), other]).unwrap()); assert_eq!( parse_board_references_for_workstream(&tags, "123e4567-e89b-12d3-a456-426614174000"), diff --git a/desktop/src/features/workstream-board/lib/boardReferences.test.mjs b/desktop/src/features/workstream-board/lib/boardReferences.test.mjs index 8998fd8e95..fbf43741f1 100644 --- a/desktop/src/features/workstream-board/lib/boardReferences.test.mjs +++ b/desktop/src/features/workstream-board/lib/boardReferences.test.mjs @@ -20,7 +20,7 @@ const ref = (kind, identity, workstreamId = WS) => ({ }, }); test("ordered mixed references round trip while malformed and cross-workstream entries fail closed", () => { - const valid = [ref("workstream", "one"), ref("pull-request", "one")]; + const valid = [ref("workstream", WS), ref("pull-request", "one")]; const tags = [ ...boardReferenceTags(valid), ["buzz:board-ref", "99", "{}"], @@ -31,9 +31,9 @@ test("ordered mixed references round trip while malformed and cross-workstream e }); test("resolution distinguishes same identity across kinds and removed objects", () => { const refs = [ - ref("workstream", "same"), ref("pull-request", "same"), - ref("agent-group", "gone"), + ref("agent-group", "same"), + ref("pull-request", "gone"), ]; const live = new Map([ [boardReferenceKey(refs[0]), refs[0]], @@ -47,3 +47,42 @@ test("resolution distinguishes same identity across kinds and removed objects", ["live", "changed", "historical"], ); }); + +test("strict decoding rejects unknown fields and Unicode controls per reference", () => { + const valid = ref("workstream", WS); + const malformed = [ + { ...valid, extra: true }, + { ...valid, snapshot: { ...valid.snapshot, extra: true } }, + { ...valid, placement: { ...valid.placement, extra: true } }, + { ...valid, snapshot: { label: "bad\u0085label" } }, + ]; + const tags = malformed.map((value) => [ + "buzz:board-ref", + "1", + JSON.stringify(value), + ]); + assert.deepEqual( + parseBoardReferences([...tags, ...boardReferenceTags([valid])]), + [valid], + ); +}); + +test("workstream identity is canonically bound to placement", () => { + const forged = ref("workstream", OTHER); + forged.placement.workstreamId = WS; + const tag = ["buzz:board-ref", "1", JSON.stringify(forged)]; + assert.deepEqual(parseBoardReferences([tag], WS), []); +}); + +test("same reusable identity is scoped to its recorded workstream", () => { + const inA = ref("agent", "6".repeat(64), WS); + const inB = ref("agent", "6".repeat(64), OTHER); + const live = new Map([ + [boardReferenceKey(inA), inA], + [boardReferenceKey(inB), inB], + ]); + assert.notEqual(boardReferenceKey(inA), boardReferenceKey(inB)); + assert.equal(resolveBoardReferences([inA], live)[0].state, "live"); + live.delete(boardReferenceKey(inA)); + assert.equal(resolveBoardReferences([inA], live)[0].state, "historical"); +}); diff --git a/desktop/src/features/workstream-board/lib/boardReferences.ts b/desktop/src/features/workstream-board/lib/boardReferences.ts index 09ea5f2f6a..3120049cd8 100644 --- a/desktop/src/features/workstream-board/lib/boardReferences.ts +++ b/desktop/src/features/workstream-board/lib/boardReferences.ts @@ -37,33 +37,56 @@ const KINDS = new Set([ "agent-group", "thread-blocker", ]); +const CONTROL_CHARACTER = /\p{Cc}/u; const safe = (value: unknown, max = 160): value is string => typeof value === "string" && value.trim().length > 0 && value.length <= max && - ![...value].some((character) => { - const code = character.charCodeAt(0); - return code < 32 || code === 127; - }); + !CONTROL_CHARACTER.test(value); + +const hasExactKeys = ( + value: Record, + required: readonly string[], + optional: readonly string[] = [], +): boolean => { + const allowed = new Set([...required, ...optional]); + return ( + required.every((key) => Object.hasOwn(value, key)) && + Object.keys(value).every((key) => allowed.has(key)) + ); +}; export function isBoardReference(value: unknown): value is BoardReference { if (!value || typeof value !== "object" || Array.isArray(value)) return false; const v = value as Record; + if (typeof v.kind !== "string" || !KINDS.has(v.kind)) return false; + const topLevelKeys = + v.kind === "thread-blocker" + ? ["kind", "identity", "destination", "snapshot", "placement"] + : ["kind", "identity", "snapshot", "placement"]; + if (!hasExactKeys(v, topLevelKeys) || !safe(v.identity, 512)) return false; if ( - typeof v.kind !== "string" || - !KINDS.has(v.kind) || - !safe(v.identity, 512) || !v.snapshot || typeof v.snapshot !== "object" || - Array.isArray(v.snapshot) || - !safe((v.snapshot as Record).label) || - ((v.snapshot as Record).detail !== undefined && - !safe((v.snapshot as Record).detail)) + Array.isArray(v.snapshot) + ) + return false; + const snapshot = v.snapshot as Record; + if ( + !hasExactKeys(snapshot, ["label"], ["detail"]) || + !safe(snapshot.label) || + (snapshot.detail !== undefined && !safe(snapshot.detail)) ) return false; const p = v.placement as Record | undefined; if ( !p || + !hasExactKeys(p, [ + "workstreamId", + "workstreamLabel", + "sectionId", + "sectionLabel", + ]) || !safe(p.workstreamId, 36) || !UUID.test(p.workstreamId) || !safe(p.workstreamLabel) || @@ -71,11 +94,13 @@ export function isBoardReference(value: unknown): value is BoardReference { !safe(p.sectionLabel) ) return false; + if (v.kind === "workstream" && v.identity !== p.workstreamId) return false; if (v.kind === "agent" && !HEX.test(v.identity)) return false; if (v.kind === "thread-blocker") { const d = v.destination as Record | undefined; if ( !d || + !hasExactKeys(d, ["channelId", "messageId"], ["threadRootId"]) || !safe(d.channelId, 36) || !UUID.test(d.channelId) || !safe(d.messageId, 64) || @@ -162,7 +187,7 @@ export function resolveBoardReferences( } export function boardReferenceKey( - reference: Pick, + reference: Pick, ): string { - return `${reference.kind}:${reference.identity}`; + return `${reference.placement.workstreamId}:${reference.kind}:${reference.identity}`; } diff --git a/desktop/src/features/workstream-board/ui/BoardReferenceItems.tsx b/desktop/src/features/workstream-board/ui/BoardReferenceItems.tsx new file mode 100644 index 0000000000..90dc089377 --- /dev/null +++ b/desktop/src/features/workstream-board/ui/BoardReferenceItems.tsx @@ -0,0 +1,37 @@ +import type { BoardReference } from "@/features/workstream-board/lib/boardReferences"; +import { boardReferenceKey } from "@/features/workstream-board/lib/boardReferences"; +import { cn } from "@/shared/lib/cn"; + +export function BoardReferenceItems({ + references, + onRemove, +}: { + references: readonly BoardReference[]; + onRemove?: (reference: BoardReference) => void; +}) { + return ( +
    + {references.map((reference, index) => ( +
  1. + + {index + 1}. {reference.kind}: {reference.snapshot.label} + + {onRemove ? ( + + ) : null} +
    2. + ))} +
+ ); +} diff --git a/desktop/src/features/workstream-board/ui/BoardReferenceSet.tsx b/desktop/src/features/workstream-board/ui/BoardReferenceSet.tsx index bf45bae897..78fec8a94e 100644 --- a/desktop/src/features/workstream-board/ui/BoardReferenceSet.tsx +++ b/desktop/src/features/workstream-board/ui/BoardReferenceSet.tsx @@ -1,5 +1,6 @@ import { useAppNavigation } from "@/app/navigation/useAppNavigation"; import { parseBoardReferences } from "@/features/workstream-board/lib/boardReferences"; +import { BoardReferenceItems } from "@/features/workstream-board/ui/BoardReferenceItems"; export const BOARD_REPLAY_STORAGE_KEY = "buzz:board-replay:v1"; @@ -26,14 +27,7 @@ export function BoardReferenceSet({ }} type="button" > - {references.map((reference, index) => ( - - {index + 1}. {reference.kind}: {reference.snapshot.label} - - ))} + ); } diff --git a/desktop/src/features/workstream-board/ui/WorkstreamBoardScreen.tsx b/desktop/src/features/workstream-board/ui/WorkstreamBoardScreen.tsx index 300144565b..11c0e71d50 100644 --- a/desktop/src/features/workstream-board/ui/WorkstreamBoardScreen.tsx +++ b/desktop/src/features/workstream-board/ui/WorkstreamBoardScreen.tsx @@ -22,8 +22,10 @@ import { } from "@/features/workstream-board/lib/workstreamWaits"; import { WorkstreamCard } from "@/features/workstream-board/ui/WorkstreamCard"; import { BOARD_REPLAY_STORAGE_KEY } from "@/features/workstream-board/ui/BoardReferenceSet"; +import { BoardReferenceItems } from "@/features/workstream-board/ui/BoardReferenceItems"; import { useIdentityQuery } from "@/shared/api/hooks"; import { Button } from "@/shared/ui/button"; +import { cn } from "@/shared/lib/cn"; import { PageHeader } from "@/shared/ui/PageHeader"; const WORKSTREAM_CARD_GRID_CLASS = @@ -59,16 +61,26 @@ export function WorkstreamBoardScreen() { BoardReference[] >([]); const [replayFocusIndex, setReplayFocusIndex] = React.useState(0); + const startReplay = React.useCallback((references: BoardReference[]) => { + setReplayFocusIndex(0); + setReplayReferences(references); + }, []); React.useEffect(() => { if (!discussionMode && replayReferences.length === 0) return; const onKeyDown = (event: KeyboardEvent) => { if (event.key === "Escape") { if (replayReferences.length > 0) setReplayReferences([]); else if ( - selectedReferences.length === 0 || - window.confirm("Discard the unsent reference tray?") - ) + (selectedReferences.length === 0 && draft.length === 0) || + window.confirm( + "Discard the unsent discussion draft and reference tray?", + ) + ) { setDiscussionMode(false); + setSelectedReferences([]); + setDiscussionChannelId(null); + setDraft(""); + } } else if ( replayReferences.length > 0 && (event.key === "ArrowRight" || event.key === "]") @@ -90,7 +102,7 @@ export function WorkstreamBoardScreen() { }; window.addEventListener("keydown", onKeyDown); return () => window.removeEventListener("keydown", onKeyDown); - }, [discussionMode, replayReferences, selectedReferences.length]); + }, [discussionMode, draft, replayReferences, selectedReferences.length]); const [liveReferencesByChannel, setLiveReferencesByChannel] = React.useState< ReadonlyMap >(() => new Map()); @@ -101,7 +113,7 @@ export function WorkstreamBoardScreen() { try { const parsed: unknown = JSON.parse(stored); if (Array.isArray(parsed)) { - setReplayReferences( + startReplay( parsed.flatMap((value) => { const references = parseBoardReferences([ ["buzz:board-ref", "1", JSON.stringify(value)], @@ -113,7 +125,7 @@ export function WorkstreamBoardScreen() { } catch { // Malformed cross-route replay state fails closed. } - }, []); + }, [startReplay]); const workstreamChannels = filterWorkstreamChannels(channelsQuery.data ?? []); const discussionChannel = workstreamChannels.find((channel) => channel.id === discussionChannelId) ?? @@ -131,6 +143,9 @@ export function WorkstreamBoardScreen() { () => new Set(replayReferences.map(boardReferenceKey)), [replayReferences], ); + const replayFocusedKey = replayReferences[replayFocusIndex] + ? boardReferenceKey(replayReferences[replayFocusIndex]) + : undefined; const liveReferences = React.useMemo( () => new Map( @@ -140,6 +155,21 @@ export function WorkstreamBoardScreen() { ), [liveReferencesByChannel], ); + const resolvedReplayReferences = React.useMemo( + () => resolveBoardReferences(replayReferences, liveReferences), + [liveReferences, replayReferences], + ); + const replayReferenceStates = React.useMemo( + () => + new Map( + resolvedReplayReferences.flatMap((item) => + item.state === "historical" + ? [] + : [[boardReferenceKey(item.reference), item.state] as const], + ), + ), + [resolvedReplayReferences], + ); const onReferencesChange = React.useCallback( (channelId: string, references: readonly BoardReference[]) => { setLiveReferencesByChannel((current) => { @@ -242,14 +272,17 @@ export function WorkstreamBoardScreen() { onClick={() => { if ( discussionMode && - selectedReferences.length > 0 && - !window.confirm("Discard the unsent reference tray?") + (selectedReferences.length > 0 || draft.length > 0) && + !window.confirm( + "Discard the unsent discussion draft and reference tray?", + ) ) return; setDiscussionMode(!discussionMode); setSelectedReferences([]); setReplayReferences([]); setDiscussionChannelId(null); + setDraft(""); }} variant={discussionMode ? "secondary" : "default"} > @@ -270,25 +303,10 @@ export function WorkstreamBoardScreen() {

Reference tray ({selectedReferences.length})

-
    - {selectedReferences.map((reference, index) => ( -
  1. - - {index + 1}. {reference.kind}: {reference.snapshot.label} - - -
    2. - ))} -
+