Skip to content

implement collaboration upgrade proposals #1856

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
giswqs merged 16 commits into from
Aug 14, 2026
Merged

implement collaboration upgrade proposals #1856

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
35879aa
implement collaboration upgrade proposals (invite roles, identity, du…
HarshShinde0 Aug 11, 2026
acf9808
style: auto-format (ruff + oxfmt) [pre-commit.ci]
pre-commit-ci[bot] Aug 11, 2026
ae9217f
fix: address review fixes
HarshShinde0 Aug 11, 2026
4956a36
style: auto-format (ruff + oxfmt) [pre-commit.ci]
pre-commit-ci[bot] Aug 11, 2026
b1f9e7b
fixes
HarshShinde0 Aug 11, 2026
43cb3d3
style: auto-format (ruff + oxfmt) [pre-commit.ci]
pre-commit-ci[bot] Aug 11, 2026
7e1e8e6
fix(collab): update rate-limit cleanup, proxy IP handling, and collab…
HarshShinde0 Aug 11, 2026
61b5cf2
style: auto-format (ruff + oxfmt) [pre-commit.ci]
pre-commit-ci[bot] Aug 11, 2026
6d39e33
fix(collab): defer invite consumption until after join authorization …
HarshShinde0 Aug 11, 2026
5238173
fix(collab): enforce provider string type safety, guard host invites,…
HarshShinde0 Aug 11, 2026
23b20ff
style: auto-format (ruff + oxfmt) [pre-commit.ci]
pre-commit-ci[bot] Aug 11, 2026
553258c
Merge branch 'main' into feat/collab-upgrade
giswqs Aug 13, 2026
1757525
Address Claude review feedback
giswqs Aug 13, 2026
a203d5f
Address Claude review feedback
giswqs Aug 13, 2026
c8fc3e1
feat(collab): add proactive layer locking UI feedback and host controls
HarshShinde0 Aug 13, 2026
4cf136d
style: auto-format (ruff + oxfmt) [pre-commit.ci]
pre-commit-ci[bot] Aug 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 36 additions & 33 deletions apps/geolibre-desktop/src/components/layout/CollaborateDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
});
};

const [requireIdentity, setRequireIdentity] = useState(false);

const handleStart = async () => {
if (!name.trim()) {
setError(t("collaborate.nameRequired"));
Expand All @@ -122,10 +124,8 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
setBusy(true);
setError(null);
try {
await api.start(name.trim(), color, mode);
await api.start(name.trim(), color, mode, requireIdentity);
} catch (err) {
// Show a localized message; keep the raw error in the console for
// diagnostics (collab-client throws human-readable English strings).
console.error("[GeoLibre] Collaboration error", err);
setError(t("collaborate.connectFailed"));
} finally {
Expand All @@ -147,12 +147,8 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
try {
await api.join(code.trim(), name.trim(), color);
} catch (err) {
// Show a localized message; keep the raw error in the console for
// diagnostics (collab-client throws human-readable English strings).
console.error("[GeoLibre] Collaboration error", err);
setError(t("collaborate.connectFailed"));

@github-actions github-actions Bot Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor UX note: this PR adds several new join-rejection reasons the relay can send back (identity-required, forbidden for a blocked participant key), each with a specific message string (e.g. "You have been blocked from this session by the host.", "Sign-in required to join this session."). api.join now rejects with new Error(message.message) carrying that specific text (see the new pendingConnectRef handling in useCollaboration.ts), but this catch block discards it in favor of the generic t("collaborate.connectFailed"). A user who gets blocked or hits an identity gate sees the same "connection failed" message as a plain wrong invite code, with no way to tell why.

This pattern predates this PR (the removed comment even called it out as intentional), so it may be out of scope — but the set of distinguishable failure reasons has grown meaningfully with this feature, which makes surfacing err.message (or at least distinguishing "blocked"/"identity-required" specifically) more valuable than it was before.

Confidence: low-medium.

// The invite link could not connect (e.g. an expired or invalid code), so
// reveal the full layout and let the user fix the code or host instead.
setInvited(false);
} finally {
setBusy(false);
Expand Down Expand Up @@ -184,13 +180,13 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
onDismiss={() => onOpenChange(false)}
onSetMode={api.setMode}
onSetParticipantMode={api.setParticipantMode}
onKickParticipant={api.kickParticipant}
onBlockParticipant={api.blockParticipant}
onSetSessionConfig={api.setSessionConfig}
onSetFollowHost={api.setFollowHost}
/>
) : (
<div className="space-y-4">
{/* Name and color feed both actions below, so group them in a
shaded panel above the cards to read as shared profile inputs
rather than belonging to either Start or Join (#706). */}
<div className="space-y-3 rounded-md border bg-muted/40 p-3">
<div className="space-y-1.5">
<Label htmlFor="collab-name">{t("collaborate.displayName")}</Label>
Expand All @@ -206,8 +202,6 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
</div>
<div className="space-y-1.5">
<Label>{t("collaborate.color")}</Label>
{/* Full panel width keeps every swatch on one row instead of
wrapping a lone dot to a second line (#706). */}
<div className="flex flex-wrap gap-2 pt-1">
{COLOR_PALETTE.map((c) => (
<button
Expand All @@ -216,9 +210,6 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
aria-label={c.name}
aria-pressed={color === c.hex}
onClick={() => setColor(c.hex)}
// Selection is an outer ring (offset from the swatch), so
// the colored circle stays the same size — a border would
// inset the fill and make the selected one look smaller.
className={`h-6 w-6 rounded-full transition ${
color === c.hex
? "ring-2 ring-offset-2 ring-offset-background ring-foreground"
Expand All @@ -231,10 +222,6 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
</div>
</div>

{/* An invited participant (arrived via a `?collab=` link) only needs
to join, so collapse the layout to a single Join action and hide
the "Start a session" controls that are irrelevant to them
(#753). They can still fall back to hosting via the link below. */}
{invited ? (
<div className="space-y-3 rounded-md border p-3">
<div className="space-y-1">
Expand Down Expand Up @@ -269,9 +256,6 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
</Button>
<button
type="button"
// Keep the prefilled `code` so the join field stays populated
// if the user changes their mind; the full layout shows Start
// as the primary action, so leaving it is non-destructive.
onClick={() => setInvited(false)}
disabled={busy}
className="cursor-pointer text-xs text-muted-foreground underline-offset-2 hover:underline disabled:cursor-default disabled:opacity-50"
Expand All @@ -295,6 +279,16 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
<option value="view-only">{t("collaborate.modeViewOnly")}</option>
</Select>
</div>
<label className="flex cursor-pointer items-center gap-2 text-xs text-muted-foreground pt-1">
<input
type="checkbox"
checked={requireIdentity}
onChange={(e) => setRequireIdentity(e.target.checked)}
disabled={busy}
className="h-3.5 w-3.5 rounded border"
/>
{(t as (key: string) => string)("collaborate.requireIdentityLabel")}
</label>

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quality/completeness: enabling "Require signed-in account to join" will lock out every guest, since there's no sign-in flow anywhere in the app.

identityToken is only ever produced by hand — searching the frontend, join(...) is called with no options (here and in CommentsPanel.tsx), so the client never actually sends an identityToken. There's no OAuth/sign-in UI, no ?collab= invite-link handling that carries a token, and mintInvite/revokeInvite/setLayerLocks (wired up in useCollaboration.ts) aren't called from anywhere in the UI either.

Net effect as shipped: a host who checks this new box will find that literally no one — including well-intentioned collaborators — can join anymore, since the app has no way to obtain a valid identity token. Same for the invite-link/layer-lock features described in the PR summary: the server/protocol/store support is all there, but there's no way for a user to reach it through the app.

Worth either wiring up minimal UI for these (invite link generation + copy, layer-lock toggles in the layer panel) before merging, or gating the "Require signed-in account" checkbox behind a flag/tooltip noting it's not yet usable, so hosts don't accidentally strand their own session.

Confidence: medium — based on searching the frontend for callers of join, mintInvite, and setLayerLocks and finding none that supply the new options.

@giswqs giswqs Aug 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 1757525. The gate can no longer strand a host: with no issuer configured, POST /sessions answers 400 and set-session-config answers a new identity-unavailable error, and the relay advertises identitySupported on /health and in every welcome so the Collaborate dialog hides the "Require signed-in account" checkbox in both the create form and the in-session host controls. GeoLibre ships no issuer, so the option is dark by default. Agreed that mintInvite/revokeInvite/setLayerLocks still have no UI — that is additive rather than a footgun (nothing a host can toggle locks them out), so leaving it for a follow-up rather than growing this PR further.

<Button
type="button"
onClick={() => void handleStart()}
Expand Down Expand Up @@ -333,9 +327,6 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
</>
)}

{/* Show local validation errors and connect failures, the latter
arriving asynchronously in the store (the WebSocket handshake
fails after this dialog's call already resolved). */}
{(error || collaboration.error) && (
<p className="rounded-md bg-destructive/10 p-2 text-sm text-destructive">
{error || collaboration.error}
Expand All @@ -356,6 +347,9 @@ function ActiveSession({
onDismiss,
onSetMode,
onSetParticipantMode,
onKickParticipant,
onBlockParticipant,
onSetSessionConfig,
onSetFollowHost,
}: {
shareLink: string;
Expand All @@ -365,6 +359,9 @@ function ActiveSession({
onDismiss: () => void;
onSetMode: (mode: CollaborationMode) => void;
onSetParticipantMode: (clientId: string, canEdit: boolean) => void;
onKickParticipant?: (clientId: string) => void;
onBlockParticipant?: (clientId: string) => void;
onSetSessionConfig?: (config: { requireIdentity?: boolean }) => void;
onSetFollowHost: (enabled: boolean) => void;
}) {
const { t } = useTranslation();
Expand All @@ -387,8 +384,6 @@ function ActiveSession({
)}
</div>

{/* Cameras are independent by default; a non-host can opt to follow the
host's viewport (presenter mode). */}
{!isHost && (
<label className="flex cursor-pointer items-center gap-2 text-sm">
<input
Expand All @@ -401,6 +396,18 @@ function ActiveSession({
</label>
)}

{isHost && (
<label className="flex cursor-pointer items-center gap-2 text-xs text-muted-foreground">
<input
type="checkbox"
checked={collaboration.requireIdentity}
onChange={(e) => onSetSessionConfig?.({ requireIdentity: e.target.checked })}
className="h-3.5 w-3.5 rounded border"
/>
{(t as (key: string) => string)("collaborate.requireIdentityLabel")}
</label>
)}

<div className="space-y-1.5">
<Label>{t("collaborate.sessionCode")}</Label>
<div className="flex gap-2">
Expand Down Expand Up @@ -441,7 +448,6 @@ function ActiveSession({
)}
</Button>
</div>
{/* Only the host invites others, so the scan-to-join QR is host-only. */}
{isHost && (
<div className="flex flex-col items-center gap-1.5 pt-1">
<div className="rounded-md bg-white p-2">
Expand Down Expand Up @@ -472,6 +478,8 @@ function ActiveSession({
isSelf={p.clientId === collaboration.clientId}
canManage={isHost}
onSetParticipantMode={onSetParticipantMode}
onKickParticipant={onKickParticipant}
onBlockParticipant={onBlockParticipant}
/>
))}
</ul>
Expand All @@ -483,12 +491,7 @@ function ActiveSession({
</p>
)}

{/* Primary way out of the dialog: dismiss it while keeping the session
live, so the host isn't tempted to use the "X" (which they fear ends
the session) to get back to the map (#754). */}
<Button type="button" className="w-full" onClick={onDismiss}>
{/* A view-only guest cannot edit, so "collaborate" would mislead; offer
"watch" wording for that case. */}
{isHost || collaboration.mode === "co-edit"
? t("collaborate.goToMap")
: t("collaborate.goToMapViewOnly")}
Expand Down
63 changes: 49 additions & 14 deletions apps/geolibre-desktop/src/components/layout/CollaborationParticipantRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { CollaborationMode, CollaborationParticipant } from "@geolibre/core";
import { Eye, Pencil } from "lucide-react";
import { Eye, Pencil, UserX, Ban } from "lucide-react";
import { useTranslation } from "react-i18next";
import { participantCanEdit } from "../../lib/collab-protocol";

Expand All @@ -14,6 +14,8 @@ interface CollaborationParticipantRowProps {
* own row never shows a control. */
canManage: boolean;
onSetParticipantMode: (clientId: string, canEdit: boolean) => void;
onKickParticipant?: (clientId: string) => void;
onBlockParticipant?: (clientId: string) => void;
/** Render the smaller variant used in the on-canvas badge roster. */
compact?: boolean;
}
Expand All @@ -30,6 +32,8 @@ export function CollaborationParticipantRow({
isSelf,
canManage,
onSetParticipantMode,
onKickParticipant,
onBlockParticipant,
compact = false,
}: CollaborationParticipantRowProps) {
const { t } = useTranslation();
Expand All @@ -51,6 +55,11 @@ export function CollaborationParticipantRow({
style={{ backgroundColor: p.color }}
/>
<span className="truncate">{p.displayName}</span>
{p.identity && (
<span className="rounded bg-primary/10 px-1 text-[10px] text-primary">
{p.identity.provider === "geolibre" ? "✓" : p.identity.provider}
</span>
)}
{isSelf && <span className="text-xs text-muted-foreground">({t("collaborate.you")})</span>}
{isHostRow && (
<span
Expand All @@ -61,19 +70,45 @@ export function CollaborationParticipantRow({
)}
{!isHostRow &&
(showToggle ? (
<button
type="button"
onClick={() => onSetParticipantMode(p.clientId, !editable)}
// A binary "can edit" vs "view-only" setting reads as a switch to
// assistive tech, rather than a momentary press (aria-pressed).
role="switch"
aria-checked={editable}
title={editable ? t("collaborate.setViewOnly") : t("collaborate.allowEdit")}
className={`ms-auto flex shrink-0 items-center gap-1 rounded border py-0.5 text-muted-foreground transition hover:bg-accent hover:text-foreground ${compact ? "px-1 text-[10px]" : "px-1.5 text-xs"}`}
>
{permIcon}
{permLabel}
</button>
<div className="ms-auto flex shrink-0 items-center gap-1">
<button
type="button"
onClick={() => onSetParticipantMode(p.clientId, !editable)}
role="switch"
aria-checked={editable}
title={editable ? t("collaborate.setViewOnly") : t("collaborate.allowEdit")}
className={`flex items-center gap-1 rounded border py-0.5 text-muted-foreground transition hover:bg-accent hover:text-foreground ${compact ? "px-1 text-[10px]" : "px-1.5 text-xs"}`}
>
{permIcon}
{permLabel}
</button>
{canManage && !compact && (
<>
{onKickParticipant && (
<button
type="button"
onClick={() => onKickParticipant(p.clientId)}
title={(t as (key: string) => string)("collaborate.kick")}
aria-label={(t as (key: string) => string)("collaborate.kick")}
className="rounded p-1 text-muted-foreground hover:bg-destructive/10 hover:text-destructive"
>
<UserX className="h-3 w-3" aria-hidden="true" />
</button>
)}
{onBlockParticipant && (
<button
type="button"
onClick={() => onBlockParticipant(p.clientId)}
title={(t as (key: string) => string)("collaborate.block")}
aria-label={(t as (key: string) => string)("collaborate.block")}
className="rounded p-1 text-muted-foreground hover:bg-destructive/10 hover:text-destructive"
>
<Ban className="h-3 w-3" aria-hidden="true" />
</button>
Comment thread
HarshShinde0 marked this conversation as resolved.
)}
</>
)}
</div>
) : (
// Non-host viewers still see each guest's current permission.
<span
Expand Down
Loading
Loading