Harden yjs container readiness and shutdown behavior - #3247
Merged
Merged
Conversation
AlexAndBear
force-pushed
the
feat/yjs-production-grade-service
branch
from
August 27, 2026 13:23
8cd0eca to
fcfa099
Compare
AlexAndBear
force-pushed
the
feat/yjs-production-grade-service
branch
from
August 29, 2026 15:47
fcfa099 to
a926a8f
Compare
AlexAndBear
requested review from
JammingBen and
kulmann
and
a lite review from Copilot
August 31, 2026 11:53
AlexAndBear
force-pushed
the
feat/yjs-production-grade-service
branch
from
August 31, 2026 13:04
617bb8a to
94485f0
Compare
JammingBen
reviewed
Aug 31, 2026
AlexAndBear
force-pushed
the
feat/yjs-production-grade-service
branch
from
August 31, 2026 14:58
0d5cd8d to
a79b14d
Compare
Contributor
There was a problem hiding this comment.
馃煛 Changes recommended
There is a shutdown-grace configuration edge case that can produce unreliable shutdown behavior, and the PR description currently claims Compose health-gating changes that aren鈥檛 present in the diff.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
services/yjs/src/server.ts:6
SHUTDOWN_GRACE_PERIOD_MSis parsed without validation. If the env var is unset/mistyped (e.g. "15s"),parseIntcan yieldNaNor a non-positive value, which makes the grace timer behavior unpredictable (including an immediate forced exit). Validate and fail fast (or clamp to a safe default) so shutdown semantics are reliable.
const SHUTDOWN_GRACE_PERIOD_MS = parseInt(process.env.SHUTDOWN_GRACE_PERIOD_MS ?? '15000', 10)
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
JammingBen
approved these changes
Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This change upgrades the
services/yjscontainer behavior for production-style operation.It adds:
HEALTHCHECKbased on actual readiness (/healthz/ready) and WebSocket reachabilitySIGTERM/SIGINT/SIGQUIT) with a documented grace periodstop_grace_periodforyjs, aligned with graceful shutdown behaviorRelated Issue
How Has This Been Tested?
pnpm --filter opencloud-yjs-server check:typespnpm exec eslint services/yjs/src/server.ts services/yjs/src/healthcheck.tspnpm exec prettier --check services/yjs/src/server.ts services/yjs/src/healthcheck.ts services/yjs/README.md docker-compose.ymldocker compose config >/tmp/docker-compose.resolved.ymlTypes of changes