mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 00:05:49 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - This change touches several already-shipped subsystems — document-comment annotations, the selected-agent Conference Room chat surface, routine description annotations, workspace-operation tracking, and the board polling/inbox UI > - A batch of incremental fixes and two small backend additions had accumulated on a local mainline and were deployed to a live instance, but never landed upstream — so each `origin/master` sync kept re-diverging > - Leaving them un-upstreamed means the same delta has to be re-merged on every sync and risks being lost or silently reverted > - This pull request rebases that delta cleanly on top of current `origin/master` (preserving recent upstream work such as reusable sandbox leases and the relation-list collapse controls) and brings it up for review > - The benefit is that mainline and the deployed instance converge, and these fixes/additions get normal review + CI + Greptile coverage ## Linked Issues or Issue Description No single GitHub issue tracks this; it is a bundle of bug fixes and two small feature additions. Following the issue-template fields: **Bug fixes (what was wrong → what this does):** - Document comments rendered out of document order, didn't live-update across clients, swallowed save failures, and lost the markdown text selection on re-render. Now: doc-order sort, live updates, surfaced save-failure state, stable selection across re-renders. - The board polling hot path returned oversized payloads on every poll. Now: an opt-in `summary` projection trims the heartbeat-run list payload. - Assorted UI fixes: sidebar nav peek/streamlining edge cases, markdown file-viewer re-mount/line-height issues on iOS Safari, inbox badge/skill deep-link tab selection, and `⌘.` work-mode cycling on iOS. **Feature additions:** - `workspace_operations.issue_id` — associate a workspace operation with the issue that triggered it (new migration `0106`, schema, service, shared type). - Routine **description annotations** — comment threads on a routine's description document, mirroring issue document annotations (new migration `0107`, `routine_documents` schema, routes/service, editable-sections UI). - Selected-agent **Conference Room chat** surface wiring and live issue-thread updates. **Related PR:** #8229 (`feat(control-plane): add annotation and workspace controls`, draft) covers overlapping annotation/workspace-control territory — flagging it so a reviewer can reconcile the two rather than double-merging. ## What Changed - `feat(workspace)`: `workspace_operations.issue_id` migration + schema/service/type; issue workspace property controls reconciled with upstream's evolved "Service" row. - `feat(routines)`: routine description annotations — `routine_documents` schema, migration `0107`, routes/service, `editable-sections` UI. - `fix(document-comments)`: doc-order sort, live updates, save-failure surfacing, stable markdown selection (+ storybook story, rerender test). - `feat(chat)`: selected-agent Conference Room chat surface and live issue-thread updates (`LiveUpdatesProvider`, `issue-chat-messages`, interactions service). - `perf(board)`: opt-in `summary` projection for the board polling/heartbeat-run list payload, plus assorted sidebar / file-viewer / inbox / IssueProperties UI fixes. Organized into 5 logical commits. Migrations are numbered incrementally after upstream's latest (`0105`) — `0106` then `0107`, no journal collision. ## Verification - Built by 3-way merging the deployed delta onto current `origin/master`; the only merge conflict (`IssueProperties.test.tsx`, two adjacent test blocks) was resolved in favor of upstream's evolved "green service link above the workspace row" layout, which matches the merged component's rendered output. - Confirmed recent upstream work is preserved post-merge: reusable sandbox lease teardown (`#8513`), the IssueProperties relation-list collapse controls, and the sidebar streamlined-nav default. - Confirmed the net diff vs `origin/master` is exactly the intended feature delta (65 files) and that overlapping server files (`issues.ts`, `agents.ts`, `heartbeat.ts`) only add feature code without disturbing upstream logic. - This delta is already running on a live deployed instance. - Full typecheck/test suite + Greptile to run in CI (see checklist). ## Risks - Two new migrations (`0106`, `0107`). Both are additive (new table / new nullable column) and ordered after upstream's `0105`; no data backfill, low risk. If another migration-bearing PR merges first, renumber before merge. - Largest blast radius is in the merged overlapping UI/service files; covered by the existing test suites for those files plus CI. - Overlaps thematically with draft PR #8229 — reviewers should reconcile rather than merge both blindly. ## Model Used Claude Opus 4.8 (`claude-opus-4-8`), extended thinking, with tool use (git, shell). Agentic coding workflow. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [ ] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
303 lines
11 KiB
TypeScript
303 lines
11 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { flushSync } from "react-dom";
|
|
import { createRoot, type Root } from "react-dom/client";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { SidebarProvider, useSidebar } from "./SidebarContext";
|
|
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
|
|
|
const COLLAPSED_STORAGE_KEY = "paperclip.sidebar.collapsed";
|
|
|
|
// Mutable media state driving the matchMedia mock.
|
|
const mediaState = { mobile: false, hoverFine: true };
|
|
|
|
function act(callback: () => void) {
|
|
flushSync(callback);
|
|
}
|
|
|
|
function setViewport({ mobile, hoverFine }: { mobile?: boolean; hoverFine?: boolean }) {
|
|
if (typeof mobile === "boolean") mediaState.mobile = mobile;
|
|
if (typeof hoverFine === "boolean") mediaState.hoverFine = hoverFine;
|
|
Object.defineProperty(window, "innerWidth", {
|
|
configurable: true,
|
|
writable: true,
|
|
value: mediaState.mobile ? 500 : 1280,
|
|
});
|
|
}
|
|
|
|
let capturedValue: ReturnType<typeof useSidebar> | null = null;
|
|
|
|
function Capture() {
|
|
capturedValue = useSidebar();
|
|
return null;
|
|
}
|
|
|
|
function renderProvider(): { root: Root; host: HTMLDivElement } {
|
|
const host = document.createElement("div");
|
|
document.body.appendChild(host);
|
|
const root = createRoot(host);
|
|
act(() => {
|
|
root.render(
|
|
<SidebarProvider>
|
|
<Capture />
|
|
</SidebarProvider>,
|
|
);
|
|
});
|
|
return { root, host };
|
|
}
|
|
|
|
describe("SidebarContext", () => {
|
|
let active: { root: Root; host: HTMLDivElement } | null = null;
|
|
|
|
beforeEach(() => {
|
|
localStorage.clear();
|
|
capturedValue = null;
|
|
setViewport({ mobile: false, hoverFine: true });
|
|
Object.defineProperty(window, "matchMedia", {
|
|
configurable: true,
|
|
writable: true,
|
|
value: vi.fn().mockImplementation((query: string) => {
|
|
const isHoverQuery = query.includes("(hover: hover)");
|
|
const matches = isHoverQuery ? mediaState.hoverFine : mediaState.mobile;
|
|
return {
|
|
matches,
|
|
media: query,
|
|
onchange: null,
|
|
addEventListener: vi.fn(),
|
|
removeEventListener: vi.fn(),
|
|
addListener: vi.fn(),
|
|
removeListener: vi.fn(),
|
|
dispatchEvent: vi.fn(),
|
|
};
|
|
}),
|
|
});
|
|
});
|
|
|
|
afterEach(() => {
|
|
if (active) {
|
|
act(() => active!.root.unmount());
|
|
active.host.remove();
|
|
active = null;
|
|
}
|
|
localStorage.clear();
|
|
});
|
|
|
|
describe("precedence: user pin > route request > default", () => {
|
|
it("defaults to expanded (collapsed=false) with no pin and no route request", () => {
|
|
active = renderProvider();
|
|
expect(capturedValue?.collapsed).toBe(false);
|
|
});
|
|
|
|
it("uses the route request when there is no user pin", () => {
|
|
active = renderProvider();
|
|
act(() => capturedValue?.setRouteRequestsCollapsed(true));
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
});
|
|
|
|
it("lets an explicit user pin override the route request", () => {
|
|
active = renderProvider();
|
|
act(() => capturedValue?.setRouteRequestsCollapsed(true));
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
|
|
// User pins expanded — this must win over the route's collapse request.
|
|
act(() => capturedValue?.setCollapsed(false));
|
|
expect(capturedValue?.collapsed).toBe(false);
|
|
|
|
// And pinning collapsed wins too.
|
|
act(() => capturedValue?.setCollapsed(true));
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
});
|
|
|
|
it("toggleCollapsed flips the effective mode and records a pin", () => {
|
|
active = renderProvider();
|
|
expect(capturedValue?.collapsed).toBe(false);
|
|
|
|
act(() => capturedValue?.toggleCollapsed());
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
expect(localStorage.getItem(COLLAPSED_STORAGE_KEY)).toBe("1");
|
|
|
|
act(() => capturedValue?.toggleCollapsed());
|
|
expect(capturedValue?.collapsed).toBe(false);
|
|
expect(localStorage.getItem(COLLAPSED_STORAGE_KEY)).toBe("0");
|
|
});
|
|
|
|
it("toggleCollapsed pins expanded when only a route request is active", () => {
|
|
active = renderProvider();
|
|
act(() => capturedValue?.setRouteRequestsCollapsed(true));
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
|
|
// Effective is collapsed (via route); toggling should flip to expanded.
|
|
act(() => capturedValue?.toggleCollapsed());
|
|
expect(capturedValue?.collapsed).toBe(false);
|
|
expect(localStorage.getItem(COLLAPSED_STORAGE_KEY)).toBe("0");
|
|
});
|
|
});
|
|
|
|
describe("forced collapse (secondary sidebar): overrides the pin, preserves preference", () => {
|
|
it("forces collapsed even when the user pinned expanded, without mutating the pin", () => {
|
|
active = renderProvider();
|
|
// User prefers expanded site-wide.
|
|
act(() => capturedValue?.setCollapsed(false));
|
|
expect(capturedValue?.collapsed).toBe(false);
|
|
expect(localStorage.getItem(COLLAPSED_STORAGE_KEY)).toBe("0");
|
|
|
|
// Entering a secondary-sidebar route forces the rail and locks it.
|
|
act(() => capturedValue?.setForceCollapsed(true));
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
expect(capturedValue?.collapseLocked).toBe(true);
|
|
// The persisted preference is untouched.
|
|
expect(localStorage.getItem(COLLAPSED_STORAGE_KEY)).toBe("0");
|
|
});
|
|
|
|
it("restores the user's preference when the force is cleared (leaving the route)", () => {
|
|
active = renderProvider();
|
|
act(() => capturedValue?.setCollapsed(false));
|
|
act(() => capturedValue?.setForceCollapsed(true));
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
|
|
// Navigating away clears the force; the expanded preference returns.
|
|
act(() => capturedValue?.setForceCollapsed(false));
|
|
expect(capturedValue?.collapsed).toBe(false);
|
|
expect(capturedValue?.collapseLocked).toBe(false);
|
|
});
|
|
|
|
it("locks the toggle while forced: toggleCollapsed is a no-op and never writes the pin", () => {
|
|
active = renderProvider();
|
|
act(() => capturedValue?.setCollapsed(false));
|
|
act(() => capturedValue?.setForceCollapsed(true));
|
|
|
|
act(() => capturedValue?.toggleCollapsed());
|
|
expect(capturedValue?.collapsed).toBe(true); // still forced
|
|
expect(localStorage.getItem(COLLAPSED_STORAGE_KEY)).toBe("0"); // pin unchanged
|
|
});
|
|
|
|
it("never forces or locks on mobile", () => {
|
|
setViewport({ mobile: true });
|
|
active = renderProvider();
|
|
act(() => capturedValue?.setForceCollapsed(true));
|
|
expect(capturedValue?.collapsed).toBe(false);
|
|
expect(capturedValue?.collapseLocked).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("persistence round-trip", () => {
|
|
it("writes '1'/'0' to localStorage on setCollapsed", () => {
|
|
active = renderProvider();
|
|
act(() => capturedValue?.setCollapsed(true));
|
|
expect(localStorage.getItem(COLLAPSED_STORAGE_KEY)).toBe("1");
|
|
act(() => capturedValue?.setCollapsed(false));
|
|
expect(localStorage.getItem(COLLAPSED_STORAGE_KEY)).toBe("0");
|
|
});
|
|
|
|
it("reads the persisted pin synchronously on first paint (no flash)", () => {
|
|
localStorage.setItem(COLLAPSED_STORAGE_KEY, "1");
|
|
active = renderProvider();
|
|
// Collapsed is already true on the very first captured render.
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
});
|
|
|
|
it("treats a missing/garbage value as unpinned", () => {
|
|
localStorage.setItem(COLLAPSED_STORAGE_KEY, "yes");
|
|
active = renderProvider();
|
|
expect(capturedValue?.collapsed).toBe(false);
|
|
// Unpinned, so a route request still applies.
|
|
act(() => capturedValue?.setRouteRequestsCollapsed(true));
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe("mobile gating", () => {
|
|
it("never reports collapsed on mobile even with a collapsed pin", () => {
|
|
localStorage.setItem(COLLAPSED_STORAGE_KEY, "1");
|
|
setViewport({ mobile: true });
|
|
active = renderProvider();
|
|
expect(capturedValue?.isMobile).toBe(true);
|
|
expect(capturedValue?.collapsed).toBe(false);
|
|
});
|
|
|
|
it("never reports peeking on mobile", () => {
|
|
localStorage.setItem(COLLAPSED_STORAGE_KEY, "1");
|
|
setViewport({ mobile: true });
|
|
active = renderProvider();
|
|
act(() => capturedValue?.setPeeking(true));
|
|
expect(capturedValue?.peeking).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("peek gating", () => {
|
|
it("peeks when collapsed on a hover-capable pointer", () => {
|
|
localStorage.setItem(COLLAPSED_STORAGE_KEY, "1");
|
|
setViewport({ mobile: false, hoverFine: true });
|
|
active = renderProvider();
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
act(() => capturedValue?.setPeeking(true));
|
|
expect(capturedValue?.peeking).toBe(true);
|
|
});
|
|
|
|
it("does not peek when expanded", () => {
|
|
setViewport({ mobile: false, hoverFine: true });
|
|
active = renderProvider();
|
|
expect(capturedValue?.collapsed).toBe(false);
|
|
act(() => capturedValue?.setPeeking(true));
|
|
expect(capturedValue?.peeking).toBe(false);
|
|
});
|
|
|
|
it("does not peek on a coarse/non-hover pointer", () => {
|
|
localStorage.setItem(COLLAPSED_STORAGE_KEY, "1");
|
|
setViewport({ mobile: false, hoverFine: false });
|
|
active = renderProvider();
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
act(() => capturedValue?.setPeeking(true));
|
|
expect(capturedValue?.peeking).toBe(false);
|
|
});
|
|
|
|
// iPadOS Safari keeps the hover/pointer media query false even with a
|
|
// trackpad attached (PAP-10725); a real cursor still emits "mouse" pointer
|
|
// events, which must unlock peek at runtime.
|
|
it("peeks once a mouse-type pointer event is seen (iPad + trackpad)", () => {
|
|
localStorage.setItem(COLLAPSED_STORAGE_KEY, "1");
|
|
setViewport({ mobile: false, hoverFine: false });
|
|
active = renderProvider();
|
|
expect(capturedValue?.collapsed).toBe(true);
|
|
|
|
// No cursor seen yet → peek stays gated despite the media query.
|
|
act(() => capturedValue?.setPeeking(true));
|
|
expect(capturedValue?.peeking).toBe(false);
|
|
|
|
// A trackpad/mouse moves: a "mouse" pointer event unlocks peek.
|
|
act(() => {
|
|
const e = new Event("pointermove");
|
|
(e as unknown as { pointerType: string }).pointerType = "mouse";
|
|
window.dispatchEvent(e);
|
|
});
|
|
expect(capturedValue?.peeking).toBe(true);
|
|
});
|
|
|
|
it("ignores touch pointer events (touch-only stays gated)", () => {
|
|
localStorage.setItem(COLLAPSED_STORAGE_KEY, "1");
|
|
setViewport({ mobile: false, hoverFine: false });
|
|
active = renderProvider();
|
|
|
|
act(() => capturedValue?.setPeeking(true));
|
|
act(() => {
|
|
const e = new Event("pointerover");
|
|
(e as unknown as { pointerType: string }).pointerType = "touch";
|
|
window.dispatchEvent(e);
|
|
});
|
|
expect(capturedValue?.peeking).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("back-compat", () => {
|
|
it("retains sidebarOpen/toggleSidebar for the drawer", () => {
|
|
active = renderProvider();
|
|
const initial = capturedValue?.sidebarOpen;
|
|
expect(typeof initial).toBe("boolean");
|
|
act(() => capturedValue?.toggleSidebar());
|
|
expect(capturedValue?.sidebarOpen).toBe(!initial);
|
|
});
|
|
});
|
|
});
|