nimmo/forgejo-auto-pr-review
Watch
1
0
Fork
You've already forked forgejo-auto-pr-review
 0
Code Issues 5 Pull requests Projects Releases Packages 1 Activity Actions

fix: raise Node address-space headroom #26

Merged
nimmo merged 2 commits from fix/node-address-space-default into main 2026-07-26 11:47:21 +01:00
Conversation 3 Commits 2 Files changed 3 +24 -2
nimmo commented 2026-07-26 11:17:41 +01:00
Owner
Copy link

Raise the default analysis-command RLIMIT_AS from 8 GiB to 16 GiB. Node 22 can reserve enough virtual address space under the 8 GiB limit that undici cannot instantiate its llhttp WebAssembly module, causing otherwise valid HTTP tests to fail with misleading fetch errors.

This remains a virtual-address limit rather than an assertion of resident-memory use. The default and override behavior are covered by unit tests, and the README documents the limit semantics.

Validation: 46 unit tests and nix flake check pass.

Raise the default analysis-command RLIMIT_AS from 8 GiB to 16 GiB. Node 22 can reserve enough virtual address space under the 8 GiB limit that undici cannot instantiate its llhttp WebAssembly module, causing otherwise valid HTTP tests to fail with misleading fetch errors. This remains a virtual-address limit rather than an assertion of resident-memory use. The default and override behavior are covered by unit tests, and the README documents the limit semantics. Validation: 46 unit tests and nix flake check pass.
fix: raise Node address-space headroom
All checks were successful
Build and publish container image / prepare-tags (push) Successful in 4s
Details
Build and publish container image / build-and-push (push) Successful in 29s
Details
AI pull-request review / review (pull_request) Successful in 1m38s
Details
691e251e2b
nimmo commented 2026-07-26 11:19:22 +01:00
Author
Owner
Copy link

Automated code review

Reviewed commit: 023e225cff769ab87ec0d4f2a09dd32990836d23

Verdict: Ready to merge

No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory.

Overall assessment

The pull request updates the default virtual address space limit (RLIMIT_AS) from 8 GiB to 16 GiB to accommodate Node 22's memory requirements for undici's llhttp WASM. The change correctly introduces a module-level constant, updates the configuration parser default, aligns the README example, and adds targeted unit tests for both the new default and explicit overrides. The initial review's documentation concern is already addressed by the inline comment in reviewer.py.

Blocking findings

None.

Important findings

None.

Suggestions

None.

Tests and validation

  • unit-tests: success

Questions

  • Does the repository guarantee Node.js 22+ as a baseline, or should this default remain conditional based on detected runtime versions to avoid breaking older environments?

Review limitations

  • Review is strictly limited to the supplied diff and documentation; actual runtime behavior with Node 22 and undici was not executed.
  • The change only affects the RLIMIT_AS default; other resource limits (CPU, processes) and Windows fallback paths remain unchanged.

Diff coverage

  • README.md: reviewed — included in a context-limited batch
  • reviewer.py: reviewed — included in a context-limited batch
  • tests/test_reviewer.py: reviewed — included in a context-limited batch
Review metadata
  • Reviewer: forgejo-ai-review v1
  • Model: qwen3.6:35b-a3b-q8_0
  • Guidance loaded: .forgejo/ai-review.yml, README.md
  • Commits considered: 2
  • Previous automated review: updated
  • Findings: 0
<!-- forgejo-ai-review:v1 --> ## Automated code review Reviewed commit: `023e225cff769ab87ec0d4f2a09dd32990836d23` ## Verdict: Ready to merge No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory. ### Overall assessment The pull request updates the default virtual address space limit (RLIMIT_AS) from 8 GiB to 16 GiB to accommodate Node 22's memory requirements for undici's llhttp WASM. The change correctly introduces a module-level constant, updates the configuration parser default, aligns the README example, and adds targeted unit tests for both the new default and explicit overrides. The initial review's documentation concern is already addressed by the inline comment in reviewer.py. ### Blocking findings None. ### Important findings None. ### Suggestions None. ### Tests and validation - unit-tests: **success** ### Questions - Does the repository guarantee Node.js 22+ as a baseline, or should this default remain conditional based on detected runtime versions to avoid breaking older environments? ### Review limitations - Review is strictly limited to the supplied diff and documentation; actual runtime behavior with Node 22 and undici was not executed. - The change only affects the RLIMIT_AS default; other resource limits (CPU, processes) and Windows fallback paths remain unchanged. ### Diff coverage - `README.md`: **reviewed** — included in a context-limited batch - `reviewer.py`: **reviewed** — included in a context-limited batch - `tests/test_reviewer.py`: **reviewed** — included in a context-limited batch <details><summary>Review metadata</summary> - Reviewer: forgejo-ai-review v1 - Model: qwen3.6:35b-a3b-q8_0 - Guidance loaded: .forgejo/ai-review.yml, README.md - Commits considered: 2 - Previous automated review: updated - Findings: 0 </details>
docs: explain reviewer memory default
All checks were successful
Build and publish container image / prepare-tags (push) Successful in 1s
Details
Build and publish container image / build-and-push (push) Successful in 13s
Details
AI pull-request review / review (pull_request) Successful in 2m15s
Details
023e225cff
nimmo commented 2026-07-26 11:21:43 +01:00
Author
Owner
Copy link

Implemented the valid documentation suggestion in commit 023e225: the constant now carries an inline explanation that Node 22 virtual-address reservations can prevent undici llhttp WASM initialization at the former 8 GiB limit. The repository has no changelog; the README already documents the new default for downstream users. All 46 unit tests still pass.

Implemented the valid documentation suggestion in commit 023e225: the constant now carries an inline explanation that Node 22 virtual-address reservations can prevent undici llhttp WASM initialization at the former 8 GiB limit. The repository has no changelog; the README already documents the new default for downstream users. All 46 unit tests still pass.
nimmo commented 2026-07-26 11:25:18 +01:00
Author
Owner
Copy link

Final review update: the only automated suggestion was implemented in 023e225 by documenting the Node 22/undici WASM rationale inline. The follow-up review covers that commit, reports unit-tests: success, and retains no blocking, important, or suggested findings. All 46 unit tests and nix flake check pass; PR #26 is ready to merge.

Final review update: the only automated suggestion was implemented in 023e225 by documenting the Node 22/undici WASM rationale inline. The follow-up review covers that commit, reports unit-tests: success, and retains no blocking, important, or suggested findings. All 46 unit tests and nix flake check pass; PR #26 is ready to merge.
nimmo merged commit 64e62aebc1 into main 2026-07-26 11:47:21 +01:00
nimmo deleted branch fix/node-address-space-default 2026-07-26 11:47:22 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
nimmo/forgejo-auto-pr-review!26
No description provided.