Repository navigation
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Read and apply runtime limits gonsole/limits.go, gonsole/program.go, gonsole/limits_internal_test.go, gonsole/limits_test.go, gonsole/CHANGELOG.md |
Adds RuntimeLimits and Env.RuntimeLimits. Main applies readable limits before running the program. Processor limits use runtime.GOMAXPROCS; memory limits convert MiB to bytes and use debug.SetMemoryLimit. Both settings only lower the limits held by Go. Tests cover parsing and application. |
Validate limits in check and serve gonsole/base.go, gonsole/exec_test.go, gonsole/limits_test.go |
check combines runtime-limit errors with optional settings-validation errors. serve returns limit errors before invoking the server. Tests cover invalid and valid settings and command behavior. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant Main
participant Env.RuntimeLimits
participant GoRuntime
participant runner.check
participant runner.serve
participant Program.Serve
Main->>Env.RuntimeLimits: read limits and apply them to GoRuntime
Main->>runner.check: run check command
runner.check->>Env.RuntimeLimits: validate configured limits
Main->>runner.serve: run serve command
runner.serve->>Env.RuntimeLimits: validate configured limits
runner.serve->>Program.Serve: start server when limits are valid
Suggested reviewers: codescene-access
Merge Risk: 
19dbd
-
No actionable merge-blocking risk remains from the reviewed runtime-limit changes.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 13.04% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: applying a program's processor and memory limits to Go. Linked Issues check [ #114] Objectives checked at head 19dbd28.
1. Read MAX_PROCS (at most runtime.NumCPU()) and MEMORY_LIMIT_MB under the program prefix, each unset unless named. Impleme…Out of Scope Changes check All changed files relate to [ #114]. gonsole/limits.go holds the new RuntimeLimits type, the RuntimeLimits method, and the apply logic. gonsole/program.go adds the Main call. gonsole/base.go adds the c…Full details: Linked Issues check
-
Explanation
[
#114] Objectives checked at head 19dbd28.- Read MAX_PROCS (at most runtime.NumCPU()) and MEMORY_LIMIT_MB under the program prefix, each unset unless named. Implemented in gonsole/limits.go: Env.RuntimeLimits calls e.Count for both names with default 0. The prefix is applied by Env.Count, which was not inspected directly.
- Hand both limits to Go in Main before the command runs, leaving Go's defaults when unset. Implemented: gonsole/program.go Main calls applyRuntimeLimits(p.Env, liveRuntime()) before signal setup and p.Run. RuntimeLimits.apply skips zero values, so unset limits leave Go's defaults.
- Refuse out-of-range values in check and serve. Implemented in gonsole/base.go: runner.check joins the RuntimeLimits error with the Validate error and returns it before plugin loading; runner.serve returns the RuntimeLimits error before calling program.Serve.
- Other commands run without the limit. Main ignores the RuntimeLimits error and applies nothing in that case, so other commands run.
Design notes, not failures: apply lowers limits only, so a tighter existing GOMAXPROCS or GOMEMLIMIT wins. The issue asks for caps, which is consistent with this. When either value is unreadable, neither is applied. The issue does not state this case, so it is a reasonable reading.
Residual uncertainty: an explicitly set 0 is read as unset by the default of 0. The issue range starts at 1. Whether Env.Count rejects an explicit 0 was not verified.
Full details: Out of Scope Changes check
-
Explanation
All changed files relate to [
#114]. gonsole/limits.go holds the new RuntimeLimits type, the RuntimeLimits method, and the apply logic. gonsole/program.go adds the Main call. gonsole/base.go adds the check and serve validation. gonsole/CHANGELOG.md documents the new settings. gonsole/exec_test.go adds a TestMain branch that runs limitsProgram as a subprocess, which supports the subprocess tests. gonsole/limits_internal_test.go and gonsole/limits_test.go test the parsing, application, and command behavior. No change was found that is unrelated to the issue. Assessment relies on the AI-generated summary of changes for the file list. The full diff was not read.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 13.04% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 23 functions across 6 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
 [Medium impact] No blocking findings remain. The changes since the previous review regroup existing tests without changing production behavior.
|
…ge the reading cases
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
Closes #114
What
Env.RuntimeLimitsreadsMAX_PROCS(1 to the number of CPUs) andMEMORY_LIMIT_MB(1 to 1073741824). Each stays unset unless named.Mainhands them toruntime.GOMAXPROCSanddebug.SetMemoryLimitbefore the command runs. Each only lowers what Go holds, so a container's CPU quota or a tighterGOMEMLIMITstill wins. It applies neither when one cannot be read, and nothing when both are unset.checkandserverefuse a value out of range, andchecknames it beside the program's other bad settings. Every other command runs without it.Why
Go reads
GOMAXPROCSandGOMEMLIMITbeforemain, and a program loads its.envinsidemain. On a host where.envis the only way in, a program had no way to cap its processors or its memory.Testing
go test -race ./...ingonsole, coverage 100%.procs 1 memory 67108864with both limits set, Go's own limits when one cannot be read, and a tighterGOMEMLIMITkept. It fails without the call inMain.go veton Windows, macOS and 386.Summary by CodeRabbit
MAX_PROCSandMEMORY_LIMIT_MB. Limits are applied when the settings can be read and only tighten existing runtime limits.checkandservecommands now report invalid runtime-limit settings before proceeding. Processor values must be between 1 and the available CPU count; memory values must be between 1 and 1,073,741,824 MiB.