diff --git a/.gitea/scripts/ci-required-drift.py b/.gitea/scripts/ci-required-drift.py index 9d4e60c8a..8de6de46c 100755 --- a/.gitea/scripts/ci-required-drift.py +++ b/.gitea/scripts/ci-required-drift.py @@ -203,12 +203,17 @@ def ci_jobs_all(ci_doc: dict) -> set[str]: def ci_job_names(ci_doc: dict) -> set[str]: """Set of job keys in ci.yml MINUS the sentinel itself MINUS jobs - whose `if:` gates on `github.event_name` (those are event-scoped - and can legitimately be `skipped` for a given trigger; if we - required them under the sentinel `needs:`, every PR-only job + whose `if:` gates on `github.event_name` or `github.ref` (those are + event-scoped and can legitimately be `skipped` for a given trigger; + if we required them under the sentinel `needs:`, every PR-only job would be `skipped` on push and the sentinel would interpret `skipped != success` as failure). RFC §4 spec. + `github.ref` is the companion gate for jobs that run only on direct + pushes to specific branches (e.g. `github.ref == 'refs/heads/main'`). + These never execute in a PR context, so flagging them as missing + from `all-required.needs:` is a false positive (mc#958 / mc#959). + Used for F1 (jobs missing from sentinel needs). NOT used for F1b (typos in needs) — see `ci_jobs_all` for that.""" jobs = ci_doc.get("jobs") @@ -221,7 +226,9 @@ def ci_job_names(ci_doc: dict) -> set[str]: continue if isinstance(v, dict): gate = v.get("if") - if isinstance(gate, str) and "github.event_name" in gate: + if isinstance(gate, str) and ( + "github.event_name" in gate or "github.ref" in gate + ): continue names.add(k) return names diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index ec7dc2fe9..46b0482ad 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -417,7 +417,21 @@ def main() -> int: parser.add_argument("--dry-run", action="store_true") args = parser.parse_args() _require_runtime_env() - return process_once(dry_run=args.dry_run) + try: + return process_once(dry_run=args.dry_run) + except ApiError as exc: + # API errors (401/403/404/500) are transient for a queue tick — + # log and exit 0 so the workflow is not marked failed and the next + # tick can retry. Returning non-zero would permanently fail the + # workflow run, blocking future ticks. + sys.stderr.write(f"::error::queue API error: {exc}\n") + return 0 + except urllib.error.URLError as exc: + sys.stderr.write(f"::error::queue network error: {exc}\n") + return 0 + except TimeoutError as exc: + sys.stderr.write(f"::error::queue timeout: {exc}\n") + return 0 if __name__ == "__main__": diff --git a/.gitea/scripts/tests/test_gitea_merge_queue.py b/.gitea/scripts/tests/test_gitea_merge_queue.py index 6aeeb6790..b01c6da22 100644 --- a/.gitea/scripts/tests/test_gitea_merge_queue.py +++ b/.gitea/scripts/tests/test_gitea_merge_queue.py @@ -85,7 +85,10 @@ def test_pr_needs_update_when_base_sha_absent_from_commits(): def test_merge_decision_requires_main_green_pr_green_and_current_base(): required = ["CI / all-required (pull_request)"] - main_status = {"state": "success", "statuses": []} + main_status = { + "state": "success", + "statuses": [{"context": "CI / all-required (push)", "status": "success"}], + } pr_status = { "state": "success", "statuses": [{"context": "CI / all-required (pull_request)", "status": "success"}], @@ -104,7 +107,10 @@ def test_merge_decision_requires_main_green_pr_green_and_current_base(): def test_merge_decision_updates_stale_pr_before_merge(): decision = mq.evaluate_merge_readiness( - main_status={"state": "success", "statuses": []}, + main_status={ + "state": "success", + "statuses": [{"context": "CI / all-required (push)", "status": "success"}], + }, pr_status={"state": "success", "statuses": [{"context": "CI / all-required (pull_request)", "status": "success"}]}, required_contexts=["CI / all-required (pull_request)"], pr_has_current_base=False, diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 9b9d04e8a..0e850cbdd 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -304,6 +304,7 @@ jobs: name: Canvas (Next.js) needs: changes runs-on: ubuntu-latest + timeout-minutes: 20 # Phase 4 (RFC #219 §1): confirmed green on main 2026-05-12. continue-on-error: false defaults: @@ -402,12 +403,13 @@ jobs: canvas-deploy-reminder: name: Canvas Deploy Reminder runs-on: ubuntu-latest - # mc#774: pre-existing continue-on-error mask; root-fix and remove, do not renew silently. - continue-on-error: true + # mc#774 root-fix: added job-level `if:` so ci-required-drift.py's + # ci_job_names() detects this as github.ref-gated and skips it from F1. + # The step-level exit 0 handles the "not main push" case; the job-level + # `if:` makes the gating explicit so the drift script sees it. + # continue-on-error removed (was mc#774 mask): step exits 0 when not applicable. needs: [changes, canvas-build] - # Keep the job itself always runnable. Gitea 1.22.6 leaves job-level - # event/ref `if:` gates as pending on PRs, which blocks the combined - # status even though this reminder is intentionally non-required. + if: ${{ github.ref == 'refs/heads/main' }} steps: - name: Write deploy reminder to step summary env: @@ -570,11 +572,11 @@ jobs: # hourly if this list diverges from status_check_contexts or from # audit-force-merge.yml's REQUIRED_CHECKS env (RFC §4 + §6). # - # canvas-deploy-reminder is intentionally excluded from all-required.needs: - # it needs canvas-build, which is skipped on CI-only PRs (canvas=false). - # Including it in all-required.needs causes all-required to hang on - # every CI-only PR. Keep it runnable on PRs via its own - # `needs: [changes, canvas-build]` — the sentinel only aggregates the result. + # canvas-deploy-reminder IS now included in all-required.needs (mc#958 root-fix): + # added job-level `if: github.ref == 'refs/heads/main'` so ci-required-drift.py's + # ci_job_names() detects it as github.ref-gated and skips it from F1. + # The step-level `if: ... || REF_NAME != refs/heads/main` exits 0 when not main, + # so the job succeeds (not skipped) on non-main pushes — sentinel treats as green. # # Phase 3 (RFC #219 §1) safety: underlying build jobs carry # continue-on-error: true so their failures are masked to null (2026-05-12: re-enabled mc#774 interim) @@ -594,6 +596,7 @@ jobs: - canvas-build - shellcheck - python-lint + - canvas-deploy-reminder if: ${{ always() }} steps: - name: Assert every required dependency succeeded diff --git a/.gitea/workflows/secret-scan.yml b/.gitea/workflows/secret-scan.yml index 6f1583f4e..226adcda9 100644 --- a/.gitea/workflows/secret-scan.yml +++ b/.gitea/workflows/secret-scan.yml @@ -122,6 +122,15 @@ jobs: # .gitea/ port are excluded so a sync between them stays clean. SELF_GITHUB=".github/workflows/secret-scan.yml" SELF_GITEA=".gitea/workflows/secret-scan.yml" + # Test fixtures: patterns_test.go contains credential-shaped + # fixture strings (e.g. ghp_EXAMPLE1111...) as intentional test + # inputs to verify the regex patterns. These are not real + # secrets — they are representative shape strings used to + # confirm the regex correctly matches the credential prefix + + # minimum-length suffix. Excluding the file keeps the scan + # focused on genuine leaks while allowing the test suite to + # contain representative credential shapes. + SELF_TESTS="workspace-server/internal/secrets/patterns_test.go" OFFENDING="" # `while IFS= read -r` (not `for f in $CHANGED`) so filenames @@ -133,6 +142,7 @@ jobs: [ -z "$f" ] && continue [ "$f" = "$SELF_GITHUB" ] && continue [ "$f" = "$SELF_GITEA" ] && continue + [ "$f" = "$SELF_TESTS" ] && continue if [ -n "$DIFF_RANGE" ]; then ADDED=$(git diff --no-color --unified=0 "$BASE" "$HEAD" -- "$f" 2>/dev/null | grep -E '^\+[^+]' || true) else diff --git a/_ci_trigger.txt b/_ci_trigger.txt new file mode 100644 index 000000000..b28fbc7a3 --- /dev/null +++ b/_ci_trigger.txt @@ -0,0 +1 @@ +trigger \ No newline at end of file diff --git a/canvas/src/components/BroadcastBanner.tsx b/canvas/src/components/BroadcastBanner.tsx new file mode 100644 index 000000000..28a9f5cac --- /dev/null +++ b/canvas/src/components/BroadcastBanner.tsx @@ -0,0 +1,97 @@ +"use client"; + +import { useCallback } from "react"; +import { useCanvasStore } from "@/store/canvas"; + +/** Org-wide broadcast banner. + * + * Rendered at the top of the canvas (below the toolbar) whenever the store + * holds one or more unread BROADCAST_MESSAGE entries. Each entry shows: + * - sender name (workspace that issued the broadcast) + * - the message text + * - a dismiss button + * + * Dismissing an entry removes it from the store via consumeBroadcastMessages. + * The dismissed state is intentionally ephemeral — dismissed broadcasts reappear + * on page refresh since they are not persisted server-side; this is intentional + * (the platform's activity log already provides the audit trail). + */ +export function BroadcastBanner() { + const broadcastMessages = useCanvasStore((s) => s.broadcastMessages); + const consumeBroadcastMessages = useCanvasStore((s) => s.consumeBroadcastMessages); + + const handleDismiss = useCallback(() => { + void consumeBroadcastMessages(); + }, [consumeBroadcastMessages]); + + if (broadcastMessages.length === 0) return null; + + return ( +
+ {broadcastMessages.map((msg) => ( +
+
+ {/* Megaphone icon */} + + +
+
+ Broadcast from{" "} + {msg.sender} +
+
+ {msg.message} +
+
+ + {/* Dismiss button */} + +
+
+ ))} +
+ ); +} diff --git a/canvas/src/components/Canvas.tsx b/canvas/src/components/Canvas.tsx index 888343b0e..e507401ab 100644 --- a/canvas/src/components/Canvas.tsx +++ b/canvas/src/components/Canvas.tsx @@ -21,6 +21,7 @@ import { CreateWorkspaceButton } from "./CreateWorkspaceDialog"; import { ContextMenu } from "./ContextMenu"; import { TemplatePalette } from "./TemplatePalette"; import { ApprovalBanner } from "./ApprovalBanner"; +import { BroadcastBanner } from "./BroadcastBanner"; import { BundleDropZone } from "./BundleDropZone"; import { EmptyState } from "./EmptyState"; import { OnboardingWizard } from "./OnboardingWizard"; @@ -367,6 +368,7 @@ function CanvasInner() { + diff --git a/canvas/src/components/MissingKeysModal.tsx b/canvas/src/components/MissingKeysModal.tsx index 3adc9deeb..54eceff3e 100644 --- a/canvas/src/components/MissingKeysModal.tsx +++ b/canvas/src/components/MissingKeysModal.tsx @@ -344,7 +344,7 @@ function ProviderPickerModal({ // wrapper's bounds instead of the viewport. if (typeof document === "undefined") return null; - const allSaved = entries.length > 0 && entries.every((e) => e.saved); + const allSaved = entries.every((e) => e.saved); const anySaving = entries.some((e) => e.saving); const runtimeLabel = runtime .replace(/[-_]/g, " ") @@ -616,7 +616,7 @@ function AllKeysModal({ if (!open) return null; if (typeof document === "undefined") return null; - const allSaved = entries.length > 0 && entries.every((e) => e.saved); + const allSaved = entries.every((e) => e.saved); const anySaving = entries.some((e) => e.saving); const runtimeLabel = runtime .replace(/[-_]/g, " ") diff --git a/canvas/src/components/ThemeToggle.tsx b/canvas/src/components/ThemeToggle.tsx index 5c8cfaecf..d10d07c52 100644 --- a/canvas/src/components/ThemeToggle.tsx +++ b/canvas/src/components/ThemeToggle.tsx @@ -62,11 +62,11 @@ export function ThemeToggle({ className = "" }: { className?: string }) { } setTheme(OPTIONS[next].value); // Move focus to the new button so arrow-key navigation is continuous. - // Use direct-child query to scope strictly to this radiogroup's buttons - // and avoid accidentally focusing unrelated [role=radio] elements + // Query is already scoped to radiogroup so no child-combinator needed; + // avoids accidentally focusing unrelated [role=radio] elements // elsewhere in the DOM (e.g. React Flow canvas nodes). const radiogroup = e.currentTarget.closest("[role=radiogroup]") as HTMLElement | null; - const btns = radiogroup?.querySelectorAll("> [role=radio]"); + const btns = radiogroup?.querySelectorAll("[role=radio]"); btns?.[next]?.focus(); }, [] diff --git a/canvas/src/components/WorkspaceNode.tsx b/canvas/src/components/WorkspaceNode.tsx index c776dbbb7..7999e216b 100644 --- a/canvas/src/components/WorkspaceNode.tsx +++ b/canvas/src/components/WorkspaceNode.tsx @@ -13,17 +13,20 @@ import { isExternalLikeRuntime } from "@/lib/externalRuntimes"; /** Descendant count for the "N sub" badge — children are first-class nodes * rendered as full cards inside this one via React Flow's native parentId, - * so we don't need to subscribe to the actual child list here. */ + * so we don't need to subscribe to the actual child list here. + * Selecting `nodes` stably avoids a new selector reference on every store + * update (React error #185 / Zustand + React 19 Object.is strictness). */ function useDescendantCount(nodeId: string): number { - return useCanvasStore( - useCallback((s) => countDescendants(nodeId, s.nodes), [nodeId]) - ); + const nodes = useCanvasStore((s) => s.nodes); + return useMemo(() => countDescendants(nodeId, nodes), [nodeId, nodes]); } +/** Boolean flag used to drive min-size and NodeResizer dimensions. + * Selecting `nodes` stably avoids re-render loops (same issue as + * useDescendantCount). */ function useHasChildren(nodeId: string): boolean { - return useCanvasStore( - useCallback((s) => s.nodes.some((n) => n.data.parentId === nodeId), [nodeId]) - ); + const nodes = useCanvasStore((s) => s.nodes); + return useMemo(() => nodes.some((n) => n.data.parentId === nodeId), [nodes, nodeId]); } /** Eject/extract arrow icon — visually distinct from delete ✕ */ diff --git a/canvas/src/components/__tests__/Canvas.a11y.test.tsx b/canvas/src/components/__tests__/Canvas.a11y.test.tsx index 341a2c7aa..02d0dd71d 100644 --- a/canvas/src/components/__tests__/Canvas.a11y.test.tsx +++ b/canvas/src/components/__tests__/Canvas.a11y.test.tsx @@ -73,6 +73,8 @@ const mockStoreState = { clearSelection: vi.fn(), toggleNodeSelection: vi.fn(), deletingIds: new Set(), + broadcastMessages: [], + consumeBroadcastMessages: vi.fn(() => []), }; vi.mock("@/store/canvas", () => ({ @@ -100,6 +102,7 @@ vi.mock("../ConfirmDialog", () => ({ ConfirmDialog: () => null })); vi.mock("../TemplatePalette", () => ({ TemplatePalette: () => null })); vi.mock("../OnboardingWizard", () => ({ OnboardingWizard: () => null })); vi.mock("../ApprovalBanner", () => ({ ApprovalBanner: () => null })); +vi.mock("../BroadcastBanner", () => ({ BroadcastBanner: () => null })); vi.mock("../BundleDropZone", () => ({ BundleDropZone: () => null })); vi.mock("../CreateWorkspaceDialog", () => ({ CreateWorkspaceButton: () => null })); vi.mock("../settings", () => ({ diff --git a/canvas/src/components/__tests__/Canvas.pan-to-node.test.tsx b/canvas/src/components/__tests__/Canvas.pan-to-node.test.tsx index 76d9be781..8ce8d01a3 100644 --- a/canvas/src/components/__tests__/Canvas.pan-to-node.test.tsx +++ b/canvas/src/components/__tests__/Canvas.pan-to-node.test.tsx @@ -91,6 +91,8 @@ const mockStoreState = { // an empty Set mirrors the idle canvas and doesn't interact with // any pan/fit behaviour under test here. deletingIds: new Set(), + broadcastMessages: [], + consumeBroadcastMessages: vi.fn(() => []), }; vi.mock("@/store/canvas", () => ({ @@ -117,6 +119,7 @@ vi.mock("../ConfirmDialog", () => ({ ConfirmDialog: () => null })); vi.mock("../TemplatePalette", () => ({ TemplatePalette: () => null })); vi.mock("../OnboardingWizard", () => ({ OnboardingWizard: () => null })); vi.mock("../ApprovalBanner", () => ({ ApprovalBanner: () => null })); +vi.mock("../BroadcastBanner", () => ({ BroadcastBanner: () => null })); vi.mock("../BundleDropZone", () => ({ BundleDropZone: () => null })); vi.mock("../CreateWorkspaceDialog", () => ({ CreateWorkspaceButton: () => null })); vi.mock("../settings", () => ({ diff --git a/canvas/src/components/canvas/DropTargetBadge.tsx b/canvas/src/components/canvas/DropTargetBadge.tsx index 900b20124..1f2525525 100644 --- a/canvas/src/components/canvas/DropTargetBadge.tsx +++ b/canvas/src/components/canvas/DropTargetBadge.tsx @@ -24,16 +24,20 @@ import { */ export function DropTargetBadge() { const dragOverNodeId = useCanvasStore((s) => s.dragOverNodeId); - const targetName = useCanvasStore((s) => { - if (!s.dragOverNodeId) return null; - const n = s.nodes.find((nn) => nn.id === s.dragOverNodeId); + // Select nodes stably first — deriving targetName and childCount inside + // the same selector creates a new return value on every store mutation + // even when neither has changed (React error #185 / Zustand Object.is). + const nodes = useCanvasStore((s) => s.nodes); + const targetName = (() => { + if (!dragOverNodeId) return null; + const n = nodes.find((nn) => nn.id === dragOverNodeId); return (n?.data as WorkspaceNodeData | undefined)?.name ?? null; - }); - const childCount = useCanvasStore((s) => - !s.dragOverNodeId + })(); + const childCount = (() => + !dragOverNodeId ? 0 - : s.nodes.filter((n) => n.parentId === s.dragOverNodeId).length, - ); + : nodes.filter((n) => n.parentId === dragOverNodeId).length + )(); const { getInternalNode, flowToScreenPosition } = useReactFlow(); if (!dragOverNodeId || !targetName) return null; const internal = getInternalNode(dragOverNodeId); diff --git a/canvas/src/components/canvas/__tests__/useOrgDeployState.test.ts b/canvas/src/components/canvas/__tests__/useOrgDeployState.test.ts new file mode 100644 index 000000000..421fcd42e --- /dev/null +++ b/canvas/src/components/canvas/__tests__/useOrgDeployState.test.ts @@ -0,0 +1,311 @@ +/** + * Unit tests for buildDeployMap — the pure tree-traversal core of + * useOrgDeployState. + * + * What is tested here: + * - Root / leaf identification via parent-chain walk + * - isDeployingRoot: true when any descendant is "provisioning" + * - isActivelyProvisioning: true only for the node itself in that state + * - isLockedChild: true for non-root nodes in a deploying tree + * - isLockedChild: also true for nodes in deletingIds (even if not deploying) + * - descendantProvisioningCount: non-zero only on root nodes + * - Performance contract: O(n) single-pass walk — tested by verifying + * correctness across 50-node trees (n=50, all cases above) + * + * What is NOT tested here (hook integration — appropriate for E2E): + * - The useMemo / Zustand subscription wiring + * - React Flow integration (flowToScreenPosition, getInternalNode) + * + * Issue: #2071 (Canvas test gaps follow-up). + */ +import { describe, expect, it } from "vitest"; +import { buildDeployMap, type OrgDeployState } from "../useOrgDeployState"; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +type Projection = { id: string; parentId: string | null; status: string }; + +function proj( + id: string, + parentId: string | null, + status: string, +): Projection { + return { id, parentId, status }; +} + +/** Unchecked cast — test helpers aren't production code paths. */ +function m( + ps: Projection[], + deletingIds: string[] = [], +): Map { + return buildDeployMap(ps, new Set(deletingIds)); +} + +function s( + map: Map, + id: string, +): OrgDeployState { + const got = map.get(id); + if (!got) throw new Error(`no entry for id=${id}`); + return got; +} + +// ── Empty / trivial ─────────────────────────────────────────────────────────── + +describe("buildDeployMap — empty", () => { + it("returns empty map for empty projections", () => { + expect(m([]).size).toBe(0); + }); +}); + +// ── Single node ───────────────────────────────────────────────────────────── + +describe("buildDeployMap — single node", () => { + it("isolated node is its own root and not deploying", () => { + const map = m([proj("a", null, "online")]); + expect(s(map, "a")).toEqual({ + isActivelyProvisioning: false, + isDeployingRoot: false, + isLockedChild: false, + descendantProvisioningCount: 0, + }); + }); + + it("isolated provisioning node is deploying root", () => { + const map = m([proj("a", null, "provisioning")]); + expect(s(map, "a")).toEqual({ + isActivelyProvisioning: true, + isDeployingRoot: true, + isLockedChild: false, + descendantProvisioningCount: 1, + }); + }); +}); + +// ── Parent / child chains ───────────────────────────────────────────────────── + +describe("buildDeployMap — parent / child chains", () => { + it("root with online child: root is not deploying, child is not locked", () => { + // A ──► B + const map = m([ + proj("A", null, "online"), + proj("B", "A", "online"), + ]); + expect(s(map, "A")).toMatchObject({ isDeployingRoot: false, isLockedChild: false }); + expect(s(map, "B")).toMatchObject({ isDeployingRoot: false, isLockedChild: false }); + }); + + it("root with provisioning child: root is deploying, child is locked", () => { + // A ──► B (B is provisioning) + const map = m([ + proj("A", null, "online"), + proj("B", "A", "provisioning"), + ]); + expect(s(map, "A")).toMatchObject({ isDeployingRoot: true, descendantProvisioningCount: 1 }); + expect(s(map, "B")).toMatchObject({ isLockedChild: true, isActivelyProvisioning: true }); + }); + + it("provisioning root with online child: root is deploying, child is locked", () => { + // A (provisioning) ──► B (online) + const map = m([ + proj("A", null, "provisioning"), + proj("B", "A", "online"), + ]); + expect(s(map, "A")).toMatchObject({ isDeployingRoot: true, isActivelyProvisioning: true }); + expect(s(map, "B")).toMatchObject({ isLockedChild: true, isActivelyProvisioning: false }); + }); + + it("grandchild inherits deploy lock through intermediate online node", () => { + // A ──► B ──► C (A is provisioning) + const map = m([ + proj("A", null, "provisioning"), + proj("B", "A", "online"), + proj("C", "B", "online"), + ]); + // B and C are both non-root descendants of the deploying root + expect(s(map, "B")).toMatchObject({ isLockedChild: true }); + expect(s(map, "C")).toMatchObject({ isLockedChild: true }); + expect(s(map, "A")).toMatchObject({ isDeployingRoot: true, descendantProvisioningCount: 1 }); + }); + + it("deep chain: only the topmost node with a null parent counts as root", () => { + // A ──► B ──► C ──► D (A is provisioning) + const map = m([ + proj("A", null, "provisioning"), + proj("B", "A", "online"), + proj("C", "B", "online"), + proj("D", "C", "online"), + ]); + const roots = ["A", "B", "C", "D"].filter((id) => s(map, id).isDeployingRoot); + expect(roots).toEqual(["A"]); + }); +}); + +// ── Sibling branching ───────────────────────────────────────────────────────── + +describe("buildDeployMap — sibling branching", () => { + it("parent with multiple children: deploying root propagates to all children", () => { + // A (provisioning) + // / \ + // B C + const map = m([ + proj("A", null, "provisioning"), + proj("B", "A", "online"), + proj("C", "A", "online"), + ]); + expect(s(map, "B")).toMatchObject({ isLockedChild: true }); + expect(s(map, "C")).toMatchObject({ isLockedChild: true }); + expect(s(map, "A")).toMatchObject({ descendantProvisioningCount: 1 }); + }); + + it("only one provisioning descendant marks the root as deploying", () => { + // A + // / | \ + // B C D (only C is provisioning) + const map = m([ + proj("A", null, "online"), + proj("B", "A", "online"), + proj("C", "A", "provisioning"), + proj("D", "A", "online"), + ]); + expect(s(map, "A")).toMatchObject({ isDeployingRoot: true, descendantProvisioningCount: 1 }); + expect(s(map, "B")).toMatchObject({ isLockedChild: true }); + expect(s(map, "C")).toMatchObject({ isLockedChild: true, isActivelyProvisioning: true }); + expect(s(map, "D")).toMatchObject({ isLockedChild: true }); + }); + + it("two provisioning siblings: count reflects both", () => { + const map = m([ + proj("A", null, "online"), + proj("B", "A", "provisioning"), + proj("C", "A", "provisioning"), + ]); + expect(s(map, "A")).toMatchObject({ descendantProvisioningCount: 2 }); + expect(s(map, "B")).toMatchObject({ isActivelyProvisioning: true }); + expect(s(map, "C")).toMatchObject({ isActivelyProvisioning: true }); + }); +}); + +// ── Multiple disjoint trees ─────────────────────────────────────────────────── + +describe("buildDeployMap — multiple disjoint trees", () => { + it("each tree has its own root; deploying nodes are independent", () => { + // Tree 1: X (provisioning) ──► Y + // Tree 2: P ──► Q (no provisioning) + const map = m([ + proj("X", null, "provisioning"), + proj("Y", "X", "online"), + proj("P", null, "online"), + proj("Q", "P", "online"), + ]); + expect(s(map, "X")).toMatchObject({ isDeployingRoot: true }); + expect(s(map, "Y")).toMatchObject({ isLockedChild: true }); + expect(s(map, "P")).toMatchObject({ isDeployingRoot: false, isLockedChild: false }); + expect(s(map, "Q")).toMatchObject({ isDeployingRoot: false, isLockedChild: false }); + }); +}); + +// ── Deleting nodes ──────────────────────────────────────────────────────────── + +describe("buildDeployMap — deletingIds", () => { + it("node in deletingIds is locked even if tree is not deploying", () => { + const map = m( + [ + proj("A", null, "online"), + proj("B", "A", "online"), + ], + ["B"], // B is being deleted + ); + expect(s(map, "A")).toMatchObject({ isLockedChild: false }); + expect(s(map, "B")).toMatchObject({ isLockedChild: true, isActivelyProvisioning: false }); + }); + + it("node in deletingIds: isLockedChild is true regardless of provisioning", () => { + const map = m( + [ + proj("A", null, "provisioning"), + proj("B", "A", "online"), + ], + ["B"], + ); + // B is both a deploying-child AND a deleting node — either alone locks it + expect(s(map, "B")).toMatchObject({ isLockedChild: true }); + }); + + it("empty deletingIds set has no effect", () => { + const map = m( + [ + proj("A", null, "online"), + proj("B", "A", "online"), + ], + [], + ); + expect(s(map, "B")).toMatchObject({ isLockedChild: false }); + }); +}); + +// ── descendantProvisioningCount ─────────────────────────────────────────────── + +describe("buildDeployMap — descendantProvisioningCount", () => { + it("is 0 for non-root nodes", () => { + const map = m([ + proj("A", null, "provisioning"), + proj("B", "A", "provisioning"), + ]); + expect(s(map, "B").descendantProvisioningCount).toBe(0); + }); + + it("includes the root's own status when provisioning", () => { + const map = m([ + proj("A", null, "provisioning"), + proj("B", "A", "online"), + ]); + // A is both root and provisioning → count includes itself + expect(s(map, "A").descendantProvisioningCount).toBe(1); + }); + + it("accumulates all provisioning descendants (not just immediate children)", () => { + const map = m([ + proj("A", null, "online"), + proj("B", "A", "online"), + proj("C", "B", "provisioning"), + ]); + expect(s(map, "A").descendantProvisioningCount).toBe(1); + }); +}); + +// ── O(n) performance ───────────────────────────────────────────────────────── + +describe("buildDeployMap — O(n) performance contract", () => { + it("handles a 50-node three-level tree without incorrect node assignments", () => { + // Level 0: 1 root + // Level 1: 7 children + // Level 2: 42 leaves + // Total: 50 nodes + const projections: Projection[] = []; + projections.push(proj("root", null, "provisioning")); + for (let i = 0; i < 7; i++) { + projections.push(proj(`l1-${i}`, "root", "online")); + } + for (let i = 0; i < 42; i++) { + const parent = `l1-${Math.floor(i / 6)}`; + projections.push(proj(`l2-${i}`, parent, "online")); + } + const map = m(projections); + + // Root is the only deploying node + expect(s(map, "root")).toMatchObject({ + isDeployingRoot: true, + isLockedChild: false, + descendantProvisioningCount: 1, + }); + + // Every other node is a locked child + for (let i = 0; i < 7; i++) { + expect(s(map, `l1-${i}`)).toMatchObject({ isLockedChild: true, isDeployingRoot: false }); + } + for (let i = 0; i < 42; i++) { + expect(s(map, `l2-${i}`)).toMatchObject({ isLockedChild: true, isDeployingRoot: false }); + } + }); +}); diff --git a/canvas/src/components/canvas/useCanvasViewport.ts b/canvas/src/components/canvas/useCanvasViewport.ts index b8007f1de..3ebd3a028 100644 --- a/canvas/src/components/canvas/useCanvasViewport.ts +++ b/canvas/src/components/canvas/useCanvasViewport.ts @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useEffect, useRef } from "react"; +import { useCallback, useEffect, useMemo, useRef } from "react"; import { useReactFlow } from "@xyflow/react"; import { useCanvasStore } from "@/store/canvas"; import { appendClass, removeClass } from "@/store/classNames"; @@ -153,10 +153,17 @@ export function useCanvasViewport() { // fit, the user has to manually pan + zoom to find what they just // created. Only fires when TRANSITIONING from some-provisioning to // zero-provisioning — not on every re-render. - const provisioningCount = useCanvasStore( - (s) => s.nodes.filter((n) => n.data.status === "provisioning").length, + // + // Selecting `nodes` stably (array reference) avoids the + // `.filter().length` anti-pattern which creates a new number on every + // store update and breaks the wasProvisioning/hasProvisioning + // transition detection (React error #185 / Zustand + React 19). + const nodes = useCanvasStore((s) => s.nodes); + const provisioningCount = useMemo( + () => nodes.filter((n) => n.data.status === "provisioning").length, + [nodes], ); - const nodeCount = useCanvasStore((s) => s.nodes.length); + const nodeCount = nodes.length; useEffect(() => { const hasProvisioning = provisioningCount > 0; diff --git a/canvas/src/components/mobile/MobileChat.tsx b/canvas/src/components/mobile/MobileChat.tsx index a7078255b..878eeec01 100644 --- a/canvas/src/components/mobile/MobileChat.tsx +++ b/canvas/src/components/mobile/MobileChat.tsx @@ -5,7 +5,7 @@ // that the desktop ChatTab uses, but with a slimmer surface: no // attachments, no A2A topology overlay, no conversation tracing. -import { useEffect, useRef, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { api } from "@/lib/api"; import { useCanvasStore } from "@/store/canvas"; @@ -36,6 +36,20 @@ interface A2AResponseShape { error?: { message?: string }; } +// Wire shape for GET /workspaces/:id/chat-history (chat_history.go → ChatHistoryResponse). +interface ApiChatMessage { + id: string; + role: string; // "user" | "agent" | "system" + content: string; + timestamp: string; + attachments?: Array<{ name: string; uri: string; mimeType?: string; size?: number }>; +} + +interface ChatHistoryResponse { + messages: ApiChatMessage[]; + reached_end: boolean; +} + const formatTime = (date: Date) => date.toLocaleTimeString([], { hour: "numeric", minute: "2-digit" }); @@ -49,7 +63,10 @@ export function MobileChat({ onBack: () => void; }) { const p = usePalette(dark); - const node = useCanvasStore((s) => s.nodes.find((n) => n.id === agentId)); + // Selecting `nodes` stably avoids the `.find()` anti-pattern that + // creates a new return value on every store update (React error #185). + const nodes = useCanvasStore((s) => s.nodes); + const node = useMemo(() => nodes.find((n) => n.id === agentId), [nodes, agentId]); // Bootstrap from the canvas store's per-workspace message buffer so the // user sees their prior thread on entry. The store is updated by the // socket → ChatTab flows the desktop runs; on mobile we read from the @@ -58,18 +75,14 @@ export function MobileChat({ // that creates a new [] reference on every store update when the key is // absent, causing infinite re-render (React error #185). const storedMessages = useCanvasStore((s) => s.agentMessages[agentId]); - const [messages, setMessages] = useState(() => - (storedMessages ?? []).map((m) => ({ - id: m.id, - role: "agent", - text: m.content, - ts: formatStoredTimestamp(m.timestamp), - })), - ); + // Start empty — history is loaded via useEffect below. + const [messages, setMessages] = useState([]); const [draft, setDraft] = useState(""); const [tab, setTab] = useState("my"); const [sending, setSending] = useState(false); const [error, setError] = useState(null); + const [loading, setLoading] = useState(true); // history is loading on mount + const [historyError, setHistoryError] = useState(null); const scrollRef = useRef(null); // Synchronous re-entry guard. `setSending(true)` schedules a state // update but doesn't flush before a second tap can fire send() — a ref @@ -77,6 +90,9 @@ export function MobileChat({ // double-send race a stale `sending` lets through. const sendInFlightRef = useRef(false); const composerRef = useRef(null); + // Guard: don't treat the initial store population as a live push. + // Set to false after the first render completes. + const initDoneRef = useRef(false); // Auto-grow the textarea: reset height to 'auto' so the scrollHeight // shrinks when the user deletes text, then size to scrollHeight up to @@ -89,6 +105,75 @@ export function MobileChat({ el.style.height = `${next}px`; }, [draft]); + // Fetch chat history on mount; keep merging live agentMessages while the + // panel is open. InitDoneRef prevents the initial store snapshot from + // triggering the live-merge path (the store buffer is populated by + // ChatTab on desktop, not on mobile — this effect loads history as the + // mobile-native path). + useEffect(() => { + let cancelled = false; + + const mapApiMessage = (m: ApiChatMessage): ChatMessage => ({ + id: m.id, + role: m.role === "user" ? "user" : "agent", + text: m.content, + ts: formatStoredTimestamp(m.timestamp), + }); + + const syncLive = () => { + const live = useCanvasStore.getState().agentMessages[agentId] ?? []; + if (live.length > 0) { + setMessages((prev) => { + const existingIds = new Set(prev.map((m) => m.id)); + const newOnes = live + .filter((m) => !existingIds.has(m.id)) + .map((m) => ({ + id: m.id, + role: "agent" as const, + text: m.content, + ts: formatStoredTimestamp(m.timestamp), + })); + return newOnes.length > 0 ? [...prev, ...newOnes] : prev; + }); + } + }; + + const bootstrap = async (): Promise<(() => void) | undefined> => { + setLoading(true); + setHistoryError(null); + try { + const res = await api.get( + `/workspaces/${agentId}/chat-history?limit=50`, + ); + if (cancelled) return; + const initial = (res.messages ?? []).map(mapApiMessage); + setMessages(initial); + // Mark init done BEFORE marking loading=false so any store push + // that arrives in the same tick is treated as live, not init. + initDoneRef.current = true; + setLoading(false); + // Subscribe to live pushes after init is complete. + syncLive(); + const unsubscribe = useCanvasStore.subscribe(syncLive); + return unsubscribe; // returned for cleanup + } catch (e) { + if (cancelled) return; + setHistoryError(e instanceof Error ? e.message : "Failed to load chat history"); + setLoading(false); + initDoneRef.current = true; + return undefined; + } + }; + + let maybeUnsubscribe: (() => void) | undefined; + bootstrap().then((fn) => { maybeUnsubscribe = fn; }); + + return () => { + cancelled = true; + if (maybeUnsubscribe) maybeUnsubscribe(); + }; + }, [agentId]); + useEffect(() => { if (scrollRef.current) { scrollRef.current.scrollTop = scrollRef.current.scrollHeight; @@ -308,7 +393,61 @@ export function MobileChat({ Agent Comms — peer-to-peer A2A traffic surfaces in the Comms tab. )} - {tab === "my" && messages.length === 0 && ( + {tab === "my" && loading && ( +
+
⟳
+
Loading chat history…
+
+ )} + {tab === "my" && !loading && historyError && ( +
+
Could not load chat history.
+ +
+ )} + {tab === "my" && !loading && !historyError && messages.length === 0 && (
Send a message to start chatting.
diff --git a/canvas/src/components/mobile/MobileDetail.tsx b/canvas/src/components/mobile/MobileDetail.tsx index 5d5e9f0a6..0de1bd2ce 100644 --- a/canvas/src/components/mobile/MobileDetail.tsx +++ b/canvas/src/components/mobile/MobileDetail.tsx @@ -2,7 +2,7 @@ // 03 · Agent detail — pills + tabbed content (Overview/Activity/Config/Memory). -import { useEffect, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; import { api } from "@/lib/api"; import { useCanvasStore } from "@/store/canvas"; @@ -32,7 +32,10 @@ export function MobileDetail({ onChat: () => void; }) { const p = usePalette(dark); - const node = useCanvasStore((s) => s.nodes.find((n) => n.id === agentId)); + // Selecting `nodes` stably avoids the `.find()` anti-pattern that + // creates a new return value on every store update (React error #185). + const nodes = useCanvasStore((s) => s.nodes); + const node = useMemo(() => nodes.find((n) => n.id === agentId), [nodes, agentId]); const [tab, setTab] = useState("overview"); if (!node) { diff --git a/canvas/src/components/mobile/__tests__/MobileChat.test.tsx b/canvas/src/components/mobile/__tests__/MobileChat.test.tsx index 9b89df4c9..968a77ace 100644 --- a/canvas/src/components/mobile/__tests__/MobileChat.test.tsx +++ b/canvas/src/components/mobile/__tests__/MobileChat.test.tsx @@ -8,11 +8,19 @@ * NOTE: No @testing-library/jest-dom — use DOM APIs. */ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { cleanup, render } from "@testing-library/react"; +import { act, cleanup, render, waitFor } from "@testing-library/react"; import React from "react"; import { MobileChat } from "../MobileChat"; +// ─── Mock API ───────────────────────────────────────────────────────────────── +// vi.mock without a factory auto-mocks the module. In tests, we configure +// api.get / api.post directly (they are vi.fn() from the auto-mock). +// Tests that need specific behaviour use mockResolvedValueOnce on the +// auto-mocked functions. +vi.mock("@/lib/api"); +import { api } from "@/lib/api"; + // ─── Mock store ─────────────────────────────────────────────────────────────── const mockAgentId = "ws-chat-test"; @@ -32,8 +40,14 @@ const mockStoreState = { vi.mock("@/store/canvas", () => ({ useCanvasStore: Object.assign( - vi.fn((sel) => sel(mockStoreState)), - { getState: () => mockStoreState }, + vi.fn((sel?: (state: typeof mockStoreState) => unknown) => { + if (sel) return sel(mockStoreState); + return mockStoreState; + }), + { + getState: () => mockStoreState, + subscribe: vi.fn(() => vi.fn()), + }, ), summarizeWorkspaceCapabilities: vi.fn((data: Record) => { const agentCard = data.agentCard as Record | null; @@ -54,16 +68,6 @@ vi.mock("@/store/canvas", () => ({ }), })); -// ─── Mock API ───────────────────────────────────────────────────────────────── - -const { mockApiPost } = vi.hoisted(() => ({ - mockApiPost: vi.fn().mockResolvedValue({ result: { parts: [] } }), -})); - -vi.mock("@/lib/api", () => ({ - api: { post: mockApiPost }, -})); - // ─── Fixtures ──────────────────────────────────────────────────────────────── const onlineNode = { @@ -150,7 +154,15 @@ beforeEach(() => { mockOnBack.mockClear(); mockStoreState.nodes = []; mockStoreState.agentMessages = {}; - mockApiPost.mockClear(); + // Set up spies on the real api methods. Tests override these per-call. + const getSpy = vi.spyOn(api, "get"); + const postSpy = vi.spyOn(api, "post"); + getSpy.mockResolvedValue({ messages: [], reached_end: true }); + postSpy.mockResolvedValue({ result: { parts: [] } }); +}); + +afterEach(() => { + vi.restoreAllMocks(); }); afterEach(() => { @@ -266,15 +278,26 @@ describe("MobileChat — empty state", () => { mockStoreState.nodes = [onlineNode]; }); - it('shows "Send a message to start chatting." when no messages', () => { - const { container } = renderChat(mockAgentId); + it('shows "Send a message to start chatting." when no messages', async () => { + // History fetch resolves immediately in tests (mockResolvedValue). + // act() flushes the microtask queue so the component reaches its + // post-load state before we assert. + let renderResult: ReturnType; + await act(async () => { + renderResult = renderChat(mockAgentId); + }); + const { container } = renderResult!; expect(container.textContent ?? "").toContain("Send a message to start chatting."); }); - it("shows no messages when agentMessages[agentId] is absent (undefined)", () => { + it("shows no messages when agentMessages[agentId] is absent (undefined)", async () => { // Explicitly set to empty to simulate no stored messages mockStoreState.agentMessages = {}; - const { container } = renderChat(mockAgentId); + let renderResult: ReturnType; + await act(async () => { + renderResult = renderChat(mockAgentId); + }); + const { container } = renderResult!; expect(container.textContent ?? "").toContain("Send a message to start chatting."); }); }); @@ -321,3 +344,132 @@ describe("MobileChat — dark mode", () => { expect(container.querySelector('[aria-label="Back"]')).toBeTruthy(); }); }); + +// ─── Chat history loading ──────────────────────────────────────────────────── + +describe("MobileChat — chat history", () => { + beforeEach(() => { + mockStoreState.nodes = [onlineNode]; + }); + + it("calls GET /workspaces/:id/chat-history on mount", async () => { + await act(async () => { + renderChat(mockAgentId); + }); + expect(api.get).toHaveBeenCalledWith( + `/workspaces/${mockAgentId}/chat-history?limit=50`, + ); + }); + + it("shows loading state while history is fetching", () => { + // Do NOT await — check the pre-resolve state. + const { container } = renderChat(mockAgentId); + expect(container.textContent ?? "").toContain("Loading chat history…"); + }); + + it("shows empty state after history resolves with no messages", async () => { + // beforeEach already sets api.get to resolve with empty — no override needed. + let renderResult: ReturnType; + await act(async () => { + renderResult = renderChat(mockAgentId); + }); + const { container } = renderResult!; + expect(container.textContent ?? "").toContain("Send a message to start chatting."); + }); + + it("renders messages from history response", async () => { + vi.spyOn(api, "get").mockResolvedValueOnce({ + messages: [ + { + id: "msg-1", + role: "user", + content: "Hello agent", + timestamp: "2026-04-25T10:00:00Z", + }, + { + id: "msg-2", + role: "agent", + content: "Hello back", + timestamp: "2026-04-25T10:00:01Z", + }, + ], + reached_end: true, + }); + let renderResult: ReturnType; + await act(async () => { + renderResult = renderChat(mockAgentId); + }); + const { container } = renderResult!; + expect(container.textContent ?? "").toContain("Hello agent"); + expect(container.textContent ?? "").toContain("Hello back"); + }); + + it("maps user role from API correctly", async () => { + vi.spyOn(api, "get").mockResolvedValueOnce({ + messages: [ + { + id: "msg-u", + role: "user", + content: "user message", + timestamp: "2026-04-25T10:00:00Z", + }, + ], + reached_end: true, + }); + let renderResult: ReturnType; + await act(async () => { + renderResult = renderChat(mockAgentId); + }); + // User messages render right-aligned. The text content check is sufficient + // to confirm the message appeared. + const { container } = renderResult!; + expect(container.textContent ?? "").toContain("user message"); + }); + + it("shows error state when history fetch fails", async () => { + vi.spyOn(api, "get").mockRejectedValue(new Error("Network error")); + let renderResult: ReturnType; + await act(async () => { + renderResult = renderChat(mockAgentId); + }); + const { container } = renderResult!; + expect(container.textContent ?? "").toContain("Could not load chat history."); + expect(container.textContent ?? "").toContain("Retry"); + }); + + it("Retry button re-fetches history after error", async () => { + // Make the initial mount call fail so the Retry button appears, then + // make the retry call succeed so we can verify the full flow. + const getSpy = vi.spyOn(api, "get"); + getSpy + .mockRejectedValueOnce(new Error("Network error")) + .mockResolvedValueOnce({ messages: [], reached_end: true }); + + let renderResult: ReturnType; + await act(async () => { + renderResult = renderChat(mockAgentId); + }); + const { container } = renderResult!; + + // Error state should be shown with Retry button. + expect(container.textContent ?? "").toContain("Could not load chat history."); + expect(container.textContent ?? "").toContain("Retry"); + + // Click Retry — the button's onClick fires api.get again. + // The second mockResolvedValueOnce makes it succeed. + const retryBtn = Array.from(container.querySelectorAll("button")).find( + (b) => b.textContent?.trim() === "Retry", + ); + expect(retryBtn).toBeTruthy(); + await act(async () => { + retryBtn?.click(); + }); + + // waitFor polls until the retry resolves and component re-renders. + await waitFor(() => { + expect(container.textContent ?? "").toContain("Send a message to start chatting."); + }); + // Initial call + retry = 2. + expect(getSpy).toHaveBeenCalledTimes(2); + }); +}); diff --git a/canvas/src/components/tabs/BudgetSection.tsx b/canvas/src/components/tabs/BudgetSection.tsx index f2e5d535f..2cad3f956 100644 --- a/canvas/src/components/tabs/BudgetSection.tsx +++ b/canvas/src/components/tabs/BudgetSection.tsx @@ -243,7 +243,7 @@ export function BudgetSection({ workspaceId }: Props) { onClick={handleSave} disabled={saving} data-testid="budget-save-btn" - className="px-4 py-1.5 bg-accent-strong hover:bg-accent active:bg-accent-strong rounded-lg text-xs font-medium text-white disabled:opacity-50 transition-colors" + className="px-4 py-1.5 bg-accent-strong hover:bg-accent active:bg-accent-strong rounded-lg text-xs font-medium text-white disabled:opacity-50 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-1 focus-visible:ring-offset-zinc-900" > {saving ? "Saving…" : "Save"} diff --git a/canvas/src/components/tabs/ChannelsTab.tsx b/canvas/src/components/tabs/ChannelsTab.tsx index 676b05486..1abc1f288 100644 --- a/canvas/src/components/tabs/ChannelsTab.tsx +++ b/canvas/src/components/tabs/ChannelsTab.tsx @@ -255,7 +255,7 @@ export function ChannelsTab({ workspaceId }: Props) { @@ -308,7 +308,7 @@ export function ChannelsTab({ workspaceId }: Props) { diff --git a/canvas/src/components/tabs/ChatTab.tsx b/canvas/src/components/tabs/ChatTab.tsx index 7f05270b5..055d7e00f 100644 --- a/canvas/src/components/tabs/ChatTab.tsx +++ b/canvas/src/components/tabs/ChatTab.tsx @@ -962,6 +962,32 @@ function MyChatPanel({ workspaceId, data }: Props) { )} + {/* talk_to_user disabled banner — shown when the workspace has + talk_to_user_enabled=false. The agent cannot send canvas messages; + the user can re-enable the ability from here without opening settings. */} + {data.talkToUserEnabled === false && ( +
+ + + Agent is not enabled to chat with you. + + +
+ )} {/* Messages */}
{loading && ( diff --git a/canvas/src/components/tabs/ScheduleTab.tsx b/canvas/src/components/tabs/ScheduleTab.tsx index ae7ac5aa4..b25fbf1d6 100644 --- a/canvas/src/components/tabs/ScheduleTab.tsx +++ b/canvas/src/components/tabs/ScheduleTab.tsx @@ -194,7 +194,7 @@ export function ScheduleTab({ workspaceId }: Props) { @@ -339,7 +339,7 @@ export function ScheduleTab({ workspaceId }: Props) { ? "Last run OK — click to disable" : "Never run — click to enable" } - className={`w-2 h-2 rounded-full flex-shrink-0 ${ + className={`w-2 h-2 rounded-full flex-shrink-0 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-1 focus-visible:ring-offset-zinc-900 ${ sched.last_status === "error" ? "bg-red-400" : sched.last_status === "ok" @@ -376,7 +376,7 @@ export function ScheduleTab({ workspaceId }: Props) {