Skip to content

better sync checks #292

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

Open
lupuszr wants to merge 1 commit into
base: main
Choose a base branch
from
Open

better sync checks #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 46 additions & 3 deletions packages/cli/src/daemon/routes/context-graph.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,52 @@ export async function handleContextGraphRoutes(ctx: RequestContext): Promise<voi

const shouldSyncSharedMemory =
(includeSharedMemory ?? includeWorkspace) !== false;

const subMap = (agent as any).subscribedContextGraphs as
| Map<string, { subscribed: boolean; synced: boolean; metaSynced?: boolean; name?: string; [k: string]: unknown }>
| undefined;
const existingSub = subMap?.get(paranetId);
const existingJobId = catchupTracker.latestByParanet.get(paranetId);
const existingJob = existingJobId ? catchupTracker.jobs.get(existingJobId) : undefined;

if (existingSub?.subscribed) {
if (existingJob && (existingJob.status === "queued" || existingJob.status === "running")) {
return jsonResponse(res, 200, {
subscribed: paranetId,
catchup: {
status: existingJob.status,
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 24, 2026

Choose a reason for hiding this comment

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

🔴 Bug: This changes the subscribe response contract from the existing "queued job" shape to arbitrary statuses (running here, synthetic done below) without returning a full catch-up result. Current callers distinguish "completed" vs "background" by the presence of peersTried/result, so these payloads will still be interpreted as "queued in background". Keep the old wire shapes here or update the typed clients/callers in the same PR.

includeWorkspace: existingJob.includeWorkspace,
jobId: existingJob.jobId,
},
});
}

if (existingSub.synced) {
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 24, 2026

Choose a reason for hiding this comment

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

🔴 Bug: existingSub.synced is not a safe proxy for "catch-up already completed". Discovery paths can set it true before a real catch-up runs, and it also doesn't distinguish durable-only sync from includeSharedMemory=true. This branch can therefore return synthetic done and skip the first real catch-up or a later SWM catch-up. Reuse only a previously successful job with matching includeWorkspace, or track catch-up completion separately from the subscription entry.

const jobId = existingJob?.jobId ?? `${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}`;
if (!existingJob) {
const syntheticJob: CatchupJob = {
jobId,
paranetId,
includeWorkspace: shouldSyncSharedMemory,
status: "done",
queuedAt: Date.now(),
startedAt: Date.now(),
finishedAt: Date.now(),
};
catchupTracker.jobs.set(jobId, syntheticJob);
catchupTracker.latestByParanet.set(paranetId, jobId);
}
return jsonResponse(res, 200, {
subscribed: paranetId,
catchup: {
status: "done",
includeWorkspace: shouldSyncSharedMemory,
jobId,
},
});
}
}

console.log(`[subscribe] contextGraph=${paranetId} includeSharedMemory=${shouldSyncSharedMemory}`);
agent.subscribeToContextGraph(paranetId);

Expand Down Expand Up @@ -1208,9 +1254,6 @@ export async function handleContextGraphRoutes(ctx: RequestContext): Promise<voi

if (job.status === "done") {
if (cleanResponse) {
const subMap = (agent as any).subscribedContextGraphs as
| Map<string, { subscribed: boolean; synced: boolean; metaSynced?: boolean; name?: string; [k: string]: unknown }>
| undefined;
const sub = subMap?.get(paranetId);
if (sub) {
sub.synced = true;
Expand Down
48 changes: 25 additions & 23 deletions packages/node-ui/src/ui/components/Modals/JoinProjectModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export function JoinProjectModal({ open, onClose, initialContextGraphId }: JoinP
const [progress, setProgress] = useState('');
const [error, setError] = useState<string | null>(null);
const [success, setSuccess] = useState(false);
const [syncInProgress, setSyncInProgress] = useState(false);
const [requestSent, setRequestSent] = useState(false);
const [sendingRequest, setSendingRequest] = useState(false);
const [accessDenied, setAccessDenied] = useState(false);
Expand All @@ -93,6 +94,7 @@ export function JoinProjectModal({ open, onClose, initialContextGraphId }: JoinP
setInviteCode(initialContextGraphId ?? '');
setError(null);
setSuccess(false);
setSyncInProgress(false);
setRequestSent(false);
setAccessDenied(false);
setProgress('');
Expand All @@ -112,6 +114,7 @@ export function JoinProjectModal({ open, onClose, initialContextGraphId }: JoinP
setJoining(true);
setError(null);
setSuccess(false);
setSyncInProgress(false);
setRequestSent(false);
setAccessDenied(false);

Expand All @@ -128,6 +131,7 @@ export function JoinProjectModal({ open, onClose, initialContextGraphId }: JoinP

setProgress('Subscribing to project…');
const subResult = await subscribeToContextGraph(cgId);
const subscribed = !!subResult?.subscribed;

setProgress('Syncing knowledge from peers…');

Expand All @@ -152,26 +156,13 @@ export function JoinProjectModal({ open, onClose, initialContextGraphId }: JoinP
}

if (catchup.status === 'timeout') {
// A poll timeout is NOT evidence of ACL denial — it just means
// no peer finished the catchup within ~90s. Common reasons:
// - project is public but peers are slow / offline,
// - network path is congested,
// - our subscribe hasn't reached a peer that holds the CG yet.
// Flipping `accessDenied` here used to push users of public
// projects straight into the "Access Restricted — send signed
// join request" flow, which is misleading and cuts them off
// from just retrying. Surface a neutral network error instead
// and let them retry; a real ACL denial lands in the `denied`
// branch above, or in the `err.message` check at the bottom
// of this function. (HEAD tier-4c G3; v10-rc's copy "syncing
// still in progress" was milder but still implied success —
// we'd rather the user retry explicitly than think the subscribe
// finished when the background sync never landed data.)
setError(
'Timed out waiting for peers to respond. The project may be slow to catch up, or no peer currently holds the data. Try again in a moment.',
);
setProgress('');
return;
if (!subscribed) {
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 24, 2026

Choose a reason for hiding this comment

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

🔴 Bug: subscribed only means the local node registered the project topics; it does not mean peer ACL checks or catch-up finished. If a slow curated project times out and later resolves to denied/failed, this branch falls through to the success state below and tells the user they joined successfully. Only treat timeout as success when the catch-up itself reached done or the refreshed project list proves the CG is locally usable.

setError(
'Timed out waiting for peers to respond. The project may be slow to catch up, or no peer currently holds the data. Try again in a moment.',
);
setProgress('');
return;
}
}

setProgress('Refreshing project list…');
Expand All @@ -184,7 +175,15 @@ export function JoinProjectModal({ open, onClose, initialContextGraphId }: JoinP
openTab({ id: `project:${joined.id}`, label: joined.name || joined.id, closable: true });
}

if (catchup.status === 'timeout') {
setSuccess(true);
setSyncInProgress(true);
setProgress('');
return;
}

setSuccess(true);
setSyncInProgress(false);
setProgress('');
// Phase 8: transition into wire-workspace step instead of
// auto-closing. The joiner can either install workspace files
Expand Down Expand Up @@ -291,10 +290,13 @@ export function JoinProjectModal({ open, onClose, initialContextGraphId }: JoinP
{success && (
<div style={{
padding: '10px 14px', borderRadius: 8, marginBottom: 16, fontSize: 12,
background: 'rgba(34, 197, 94, 0.08)', border: '1px solid rgba(34, 197, 94, 0.25)',
color: 'var(--accent-green)',
background: syncInProgress ? 'rgba(59, 130, 246, 0.08)' : 'rgba(34, 197, 94, 0.08)',
border: syncInProgress ? '1px solid rgba(59, 130, 246, 0.25)' : '1px solid rgba(34, 197, 94, 0.25)',
color: syncInProgress ? 'var(--accent-primary, #3b82f6)' : 'var(--accent-green)',
}}>
Successfully joined! Syncing knowledge from peers…
{syncInProgress
? 'Project joined successfully. Initial sync is still in progress and data will appear as peers respond.'
: 'Successfully joined! Syncing knowledge from peers…'}
</div>
)}

Expand Down
Loading