fix(workspace): consume targeted live globe state #761
No Reviewers
Dismiss Review
Are you sure you want to dismiss this review?
Labels
Clear labels
accessibility
all-platforms
area:agents
area:ai
area:chain
area:ci
area:client
area:daemon
area:p2p
area:project
area:protocol
area:runtime
area:storage
bug
chore
community
dependencies
discussion
discussion/Announcements
discussion/General
discussion/Ideas
discussion/Q&A
discussion/Show and tell
discussion/answered
documentation
duplicate
enhancement
good first issue
help wanted
i18n
infra
invalid
needs:decision
needs:hardware
paired
question
research
security
status:in-review
test
testing
tracking
wontfix
Barrier affecting people with disabilities
Run every platform's packaging and checks on this pull request, not only the three main ones
MCP server, skill, and thinking on the pool
Planner and model providers
Blockchain, SISY, verification
Continuous integration and build workflows
Desktop and web clients
sisyphusd coordinator, worker, scheduler
libp2p hosts, relays, discovery, connections between nodes
Project planning, roadmap, and governance
Protobuf schemas and RPC contracts
Workload execution and environments
Content-addressed storage, IPFS, Kubo, Cluster, S3, names, pinning
Something isn't working
Repository health, templates, Discussions, contributor experience
Updates to actions, Go modules or npm packages
Migrated from GitHub Discussions
GitHub Discussions category: Announcements
GitHub Discussions category: General
GitHub Discussions category: Ideas
GitHub Discussions category: Q&A
GitHub Discussions category: Show and tell
Q&A discussion with an accepted answer
Improvements or additions to documentation
This issue or pull request already exists
New feature or request
Good for newcomers
Extra attention is needed
Translations and right-to-left layouts
Build, CI, releases
This doesn't seem right
Waits on a decision from both owners
Waits on a machine we do not have yet
Built together by both maintainers, with co-authored commits
Further information is requested
Needs investigation or a design decision
Sealing, keys, access control, tokens, the security model
Built and in an open pull request; closes when it merges
Tests, test harnesses and the rehearsal
Roadmap and meta issues
This will not be worked on
type
Feature
A request, idea, or new functionality (GitHub issue type)
type
Task
A specific piece of work (GitHub issue type)
Milestone
No items
No Milestone
V0: prove the loop
Projects
Clear projects
No projects
Sisyphus roadmap
Done
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Sisyphus/Sisyphus#761
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
NODEstream for the owner-only globe pin,WatchWorkersfor worker details, and the node snapshot for the public city estimate.GetNodeLocation,GetNodeInfo, andListWorkersrequests on every node/peer snapshot; retain one-shot fallbacks only while a stream is unavailable.Part of #741; this does not complete its broader multi-client acceptance criteria.
Verification
npm --prefix apps/sisyphus run typechecknpm --prefix apps/sisyphus testnpm --prefix apps/sisyphus run buildmake lintactionlintgit diff --checkWorkspace globe data now follows the targeted live sources: the authenticated shared
NODEstream for the owner pin,WatchWorkersfor worker metadata, and the existing node snapshot for the public city estimate. This removes per-snapshotGetNodeInfo/GetNodeLocation/ListWorkerscalls and keeps one-shot recovery only for stream outages. This is a focused follow-up under #741; the broader client-sync acceptance is still open. /cc @rvnminers-A-and-NFollow-up validation caught and fixed an additional conversion gap: browser
NodeSnapshotmapping was droppingcityEstimatefrom both the initial GetNodeInfo response and WatchNodeState updates. That field now survives both conversion paths (and the dev proxy type), so the Workspace can actually consume it. Re-ran typecheck, all client tests, build, lint, actionlint and diff checks successfully.Refreshed this PR onto current
dev(including #748), resolving the shared-state enum overlap by preserving bothNODEandMODEL_PROFILES. Validation on the refreshed branch passed: client typecheck, all 235 client tests, production build,make lint,actionlint, andgit diff --check. Fresh CI is now running./cc @rvnminers-A-and-N
Coordination note: #769 extends
shared-state-domain.tswith theMODEL_CATALOGmapping (domain 16) on top of the #741 stream stack; this PR adds theNODEmapping (domain 1). There is no semantic collision, but the merged helper needs to retain both additions. I will leave this branch untouched and reconcile the two after the current CI/review sequence settles. /cc @rvnminers-A-and-NReally like where this lands. There are no more GetNodeLocation/GetNodeInfo/ListWorkers calls on every peer tick. I checked that the NODE stream carries
owner_locationfor the token holder and that pin edits firelocationChanged(), so the pin stays live.Two things before it can go in:
devagain: #749 and #750 touchedshared-state-domain.tsand its test.case 1: case 'SHARED_STATE_DOMAIN_NODE': return 'node'block appears twice (lines 7-10). It looks like a leftover from the last conflict resolution. It's harmless, but the second copy is dead code andno-duplicate-casewould flag it.Since the whole #741 stack also edits that switch, do you want to land this first and rebase the stack on it, or fold the NODE mapping into the bottom of the stack? Also, on a node that isn't coordinating, does
WatchWorkersend with an error? If so, the workspace would retry it every 30 s while visible. I wanted to check whether that's expected.I folded the workspace worker feed into the authenticated shared-state stream as well. When a node has no worker/pool service,
WORKERSnow reports one unavailable-domain event and keeps the stream open; a later successful pool join wakes it, instead of the page retryingWatchWorkersevery backoff interval. Transport failures still use bounded reconnect, with a one-shot worker-list fallback only during the outage.I also rebased this to a clean, linear branch on current
dev. The shared-state domain mapper now has exactly one NODE case (it was duplicated in the old conflict resolution). The renderer receives the per-domain error without triggering a reconnect loop, so no-coordinator worker availability is expected and handled./cc @rvnminers-A-and-N
Rebased the three workspace live-state commits onto current
dev. The overlap with the merged shared-state enum is resolved while retaining bothNODEandLOCATION_DISCOVERY; the duplicateNODEcase is absent. The focused shared-state-domain test passes. Fresh CI is starting; the temporary checkout has no standalone dependency install, so I did not claim a full renderer test run. /cc @rvnminers-A-and-NThanks for the careful review. I checked the current
devhead (84ef65ec, including #749/#750):shared-state-domain.tsthere has noNODEmapping, and this PR adds exactly onecase 1/SHARED_STATE_DOMAIN_NODEmapping. I also searched the PR diff and don't see a duplicate case; the earlier copy appears to have been removed during the restack.On ordering, I would land this focused workspace PR directly on
devfirst, then restack the #741 stream stack on top. It is independently green and fixes the live workspace globe state; the stack is a separate series and should not need to be a prerequisite for it. I won't merge the daemon-backed stream stack before you review it.WatchWorkerson a node without a coordinator ends withFailedPrecondition(apps/sisyphusd/api/local.go, with an explicitTestWatchWorkersRequiresAccessAndCoordinator). In this PR, that triggers one compatibilityListWorkersfallback and a bounded reconnect backoff (500 ms up to 30 s), not a fixed 30-second polling loop; once a stream update arrives the fallback is suppressed. The owner's pin has the same one-shot fallback behavior. The AccessPage draft dirty-state concern you raised on #753 is already addressed in the current PR diff by comparing effective draft values with the last saved config (accessDraftIsDirty), so reverting a toggle clears dirty state and remote updates replace a clean draft. #762 carries the separate coverage fixes you mentioned. Could you confirm you're comfortable with #761 landing first, and review the streaming stack when you have a chance?/cc @rvnminers-A-and-N