Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 16, 2026

After removing _worker_id to fix race conditions, three issues remained: incorrect register_filesystem parameters causing TypeError, misleading log labels, and missing request timeouts in tests.

Changes

  • socket_manager.py: Fixed register_filesystem call to use fs_type parameter instead of provider_type, removed invalid root_path argument
  • zndraw.py: Updated log messages to use "sid" instead of "worker_id" label (extension and filesystem registration)
  • test_job_endpoints.py: Added timeout=10 to all 9 requests calls to prevent hanging tests
# Before
self.zndraw.api.register_filesystem(
    name=name,
    provider_type=provider.__class__.__name__,  # Wrong parameter name
    root_path=provider.root_path,                # Invalid parameter
    socket_manager=self,
    public=fs["public"],
)

# After
self.zndraw.api.register_filesystem(
    name=name,
    fs_type=provider.__class__.__name__,
    socket_manager=self,
    public=fs["public"],
)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@coderabbitai
Copy link

coderabbitai bot commented Jan 16, 2026

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Comment @coderabbitai help to get the list of available commands and usage tips.

… labels, and add request timeouts

Co-authored-by: PythonFZ <46721498+PythonFZ@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix race condition in worker ID handling fix: correct API call parameters and logging labels after worker ID removal Jan 16, 2026
Copilot AI requested a review from PythonFZ January 16, 2026 12:47
@PythonFZ PythonFZ marked this pull request as ready for review January 16, 2026 15:33
@PythonFZ PythonFZ merged commit cdb5558 into fix/remove-worker-id-race-condition Jan 16, 2026
2 of 3 checks passed
PythonFZ added a commit that referenced this pull request Jan 19, 2026
* fix: remove _worker_id to fix race condition in job processing

The client was storing _worker_id from server responses, but this caused
a race condition during socket reconnection:

1. Socket reconnects with NEW sid
2. Server assigns pending job to NEW sid
3. Socket event handler receives job:assign
4. But _worker_id still holds OLD value (not yet updated)
5. Worker sends wrong worker_id to server → 400 BAD REQUEST

The fix removes _worker_id entirely and always uses socket.sio.sid
directly. This is safe because:
- The server always assigns jobs to the socket's current sid
- The socket that receives job:assign is always the one with that sid
- socket.sio.sid always reflects the current connection

Changes:
- Remove _worker_id field from ZnDraw dataclass
- Simplify sid property to return socket.sio.sid directly
- Remove worker_id storage in socket_manager._register_extensions_after_join
- Update tests to use vis.sid instead of vis._worker_id

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* refactor: remove unused workerId return from registration methods

The register_extension() and register_filesystem() methods in
api_manager.py were returning workerId from server responses, but
callers no longer use this value after removing _worker_id storage.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix: correct API call parameters and logging labels after worker ID removal (#857)

* Initial plan

* fix: address review comments - update register_filesystem params, log labels, and add request timeouts

Co-authored-by: PythonFZ <46721498+PythonFZ@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: PythonFZ <46721498+PythonFZ@users.noreply.github.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix: capture workerId from room:join to fix Socket.IO SID mismatch

- Add workerId to room:join response (single source of truth)
- Capture _worker_id on connect and reconnect
- Remove redundant _worker_id captures from registration methods (DRY)
- Fix filesystem re-registration keys (fs_entry['fs'], fs_entry['name'])

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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