Skip to content

better sync checks#292

Open
lupuszr wants to merge 1 commit intomainfrom
cg-join-fixes
Open

better sync checks#292
lupuszr wants to merge 1 commit intomainfrom
cg-join-fixes

Conversation

@lupuszr
Copy link
Copy Markdown
Contributor

@lupuszr lupuszr commented Apr 24, 2026

No description provided.

});
}

if (existingSub.synced) {
Copy link
Copy Markdown

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.

);
setProgress('');
return;
if (!subscribed) {
Copy link
Copy Markdown

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.

return jsonResponse(res, 200, {
subscribed: paranetId,
catchup: {
status: existingJob.status,
Copy link
Copy Markdown

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants