Add keepAlive flag to prevent scale-to-zero during process execution - #176
Merged
Conversation
Introduces new lifecycle features to the sandbox API, including the ability to force stop processes and retrieve the current sandbox status. The `/stop` endpoint allows for immediate or scheduled removal of the keepAlive flag from running processes, enabling auto-hibernation. The `/status` endpoint provides information on the current state of the sandbox and active keepAlive processes. Additionally, integrates the lifecycle management with the MCP tools, enhancing the overall process control and monitoring capabilities. The scale-to-zero functionality is also improved with crash recovery mechanisms. Updates include: - New `LifecycleHandler` for managing lifecycle operations. - API documentation updates for new endpoints. - Integration of keepAlive functionality in process management. - Comprehensive tests for lifecycle features and MCP integration.
This commit removes the `/stop` and `/status` endpoints from the sandbox API, along with the associated `LifecycleHandler` and related data structures. The lifecycle management features, including the ability to force stop processes and retrieve the current sandbox status, have been deprecated. Updates include: - Deletion of lifecycle-related API routes and handlers. - Removal of lifecycle management types and structures from the codebase. - Adjustments to documentation to reflect the removal of these features. This change simplifies the API and focuses on core process management functionalities.
This commit introduces functionality to manage timeouts for processes with the keepAlive flag enabled. If a process is restarted and keepAlive is active with a specified timeout, a goroutine is initiated to monitor the timeout and kill the process if it exceeds the limit. For processes with an infinite timeout, the goroutine simply waits for the process to complete. This enhancement improves process management and ensures better resource handling.
…andbox into cdrappier/keep-alive
…-zero warnings and timeout messages. Clear KeepAlive state before killing processes to prevent double ScaleEnable calls. Refactor related log messages for consistency.
…KeepAlive events. Enhance clarity by including process details in log messages for scale-to-zero operations and timeout handling. This improves consistency and debuggability of process management logs.
… (CWE-117) Add sanitizeLogValue() helper that escapes newlines and control characters. Replace structured logging (logrus.WithFields) with simple logrus.Infof/Warnf calls that use sanitized values - clearer and more readable. Addresses all CodeQL 'Log entries created from user input' warnings. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Replace strings.NewReplacer with byte-level filtering that strips all control characters (< 0x20) including newlines. This provides more thorough sanitization against log injection. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ion (CWE-117) Replace custom byte-level loop with package-level strings.NewReplacer variable (logSanitizer). CodeQL explicitly recognizes strings.Replacer.Replace as a sanitizer for go/log-injection since github/codeql#11910. Call logSanitizer.Replace() directly at each log site. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…117) CodeQL's SafeFormatArgumentSanitizer explicitly recognizes %q as safe because it escapes newline characters. This is the simplest and most idiomatic fix - no helper functions or variables needed. Removes the logSanitizer variable entirely. User-provided values (name, command) are now logged with %q which produces Go-syntax quoted strings, making any control characters visible in the output. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Reverts commits 6902a62, 16773fb, 4eeb6f7, 3a5b776 which added log sanitization for CWE-117. Keeps all other agent recommendations (structured logging with logrus.WithFields, race condition fix, etc). Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- StopProcess (SIGTERM path) now clears KeepAlive and calls ScaleEnable, preventing the scale-to-zero counter from leaking when a keepAlive process is gracefully stopped. - All reads/writes of process.KeepAlive are now synchronized via pm.mu: KillProcess and StopProcess write under Lock(), completion goroutines read under RLock(). This eliminates the data race between the kill/stop path and the completion goroutine. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Protect the read of oldProcess.KeepAlive and oldProcess.Timeout in restartProcess with pm.mu.RLock(), matching the synchronization used by KillProcess/StopProcess which write under pm.mu.Lock(). Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
LGTM
The data race flagged in the previous review is fixed — restartProcess now reads oldProcess.KeepAlive and oldProcess.Timeout under pm.mu.RLock(), consistent with how KillProcess/StopProcess write under pm.mu.Lock(). All three race conditions from the review cycle are resolved.
Tag @mendral-app with feedback or questions. View session
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.
Summary
Adds a
keepAliveflag to process execution that disables sandbox auto-hibernation (scale-to-zero) while a process runs. Uses a counter-based system viablaxel.ScaleDisable/ScaleEnable.Key changes:
blaxelpackage (src/lib/blaxel/blaxel.go) for scale-to-zero file-based counter control with file lockingkeepAliveandtimeoutparameters on process request/response (HTTP + MCP)timeout=0= infinite)ScaleReset()on startup clears stale counterLIFECYCLE.mddocumentationBug fixes (latest commits):
StopProcess(SIGTERM path) now clearsKeepAliveand callsScaleEnable, preventing the counter from leaking when a keepAlive process is gracefully stoppedprocess.KeepAliveare synchronized viapm.mu(Lock/RLock) to eliminate the data race betweenKillProcess/StopProcessand the completion goroutinerestartProcessreadsKeepAlive/Timeoutunderpm.mu.RLock()to match the locking disciplineReview & Testing Checklist for Human
StopProcessorKillProcessclearsKeepAlive=falseunder lock, the completion goroutine (which reads underRLock) correctly skips its ownScaleEnablecall — no path should decrement the counter twiceKeepAliveis restored under lock — confirm this doesn't leave the counter in a bad state if the process already exited between the check and the killwriteWithLockusesLOCK_EXbutGetCounterreads without a lock — verify this is acceptable (currently only used for logging)isScaleAvailablethread safety: ThescaleAvailableChecked/scaleAvailablebools are plain globals written once at first call — confirm no goroutine can race on the initial checkkeepAlive=true, verify counter increments, then stop/kill and verify counter returns to 0. Test withtimeout=0(infinite) and with a short timeout that expiresSuggested test plan:
ScaleResetresets counter to 0Notes