Skip to content

Report cgroup limits in computer_info and huskinfo - #131

Merged
Hotragn merged 2 commits into
Hotragn:mainfrom
Audgui-Byte:fix/computer-info-cgroup-limits
Sep 21, 2026
Merged

Hotragn merged 2 commits into
Hotragn:mainfrom
Audgui-Byte:fix/computer-info-cgroup-limits

Conversation

@Audgui-Byte

Copy link
Copy Markdown
Contributor

What does this change?

For the case in #119 (10 host CPUs, a 2-CPU / 2-GiB container), both the shared fallback probe and sandbox huskinfo now report cpus 2 and memory 2048MB.

Read finite cgroup v2 limits, with v1 CPU and memory fallbacks, and use the smaller of host capacity and the limit. Preserve fractional CPU quotas and zero-byte memory limits. Unlimited, missing and malformed limits fall back to host information, or ? when neither source is available. Memory no longer requires free. Failed resource-file reads do not stop the sandbox script.

Why?

Host /proc values can overstate what the container can use. Agent and MCP already share the core probe; the separate sandbox script needs the same behavior. A Linux-only shell fixture matrix exercises both paths to detect drift.

Fixes #119. Scope is the standard cgroup namespace root. This does not resolve arbitrary /proc/self/cgroup memberships, stricter ancestor limits or additional cpuset constraints beyond the host count from nproc.

How to test

npm run build:packages
npm run typecheck
npm test
npm run drift

Windows / Node 24.19.0: all 11 package builds and workspace typechecks passed; tests: 1,677 passed, 34 skipped, 0 failed. The host NO_COLOR variable was removed only from the child test environment to allow the existing FORCE_COLOR test to run. Drift checks passed.

The 34 skips include the 12 new Linux-only cases. Separately executed the actual shell probes with redirected artificial cgroup/proc files under Git sh: 12 conditions × 2 paths passed, including v1 separate/combined CPU mounts, fractional and above-host limits, unlimited, zero, malformed and missing inputs. An independent fixture runner also passed 24 executions and reproduced the original 10 / unknown output. These are shell-fixture checks, not live Docker tests; Docker and WSL are unavailable on this host.

Checklist

  • Package builds, typechecks and tests pass under the environment noted above
  • Both changed shell entrypoints executed against artificial resource files
  • New code follows the build contract
  • No new dependencies
  • Live Docker reproduction (not available on this host)

@vercel

vercel Bot commented Sep 19, 2026

Copy link
Copy Markdown

@Audgui-Byte is attempting to deploy a commit to the hotragn's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added the pkg: core Touches packages/core label Sep 19, 2026
@Hotragn

Hotragn commented Sep 19, 2026

Copy link
Copy Markdown
Owner

You took the follow-up and went further than it asked. The two Vercel checks are ours again — see #130, which documents exactly that so the next person is not misled — and I have approved the workflow run, so the real matrix has gone green.

I verified this against real cgroups rather than reading it, because the whole point of #119 is that the old code looked plausible and reported the host.

In a container limited the way husk limits one:

$ docker run --rm -i --cpus=0.5 --memory=512m alpine sh -s < probe.sh
cpus      0.5
memory    512MB

$ # what the old probe reported in that same container
cpus      10
memory    7724MB

That is the bug, closed. The fractional CPU survives as 0.5 rather than rounding to 1, which is the %.6g doing real work — a container pinned to half a core reporting 1 would be a different wrong answer rather than a fix.

And the two implementations agree. I ran sandbox/huskinfo.sh in the same constrained container and diffed it against the fallback probe: byte-identical. That mattered more than it might look. This repo has a recurring defect where one fact has two homes and they drift — husk.sh lived in four user agents, a version literal in three constants — so adding a second copy of a probe was the risk in this change, and your test asserting both paths produce the same output is the thing that makes it safe. That is the right instinct and it is the reason I am comfortable with the duplication.

On an unconstrained host (WSL2, no cgroup limits) the probe returns 10 / 7724MB, matching nproc and free -m exactly, so there is no regression on the common path.

The v1 unlimited sentinel is the detail I would have missed. 9223372036854771712 is not obviously a sentinel until you have been bitten by a machine advertising exabytes, and treating anything at or above 2^60 as unbounded is the right shape — a threshold rather than an equality test, since the exact value is page-size dependent.

One thing I would change, not blocking

Several assertions match the source text of the shell one-liner:

expect(PROBE_COMPUTER_INFO).toContain('quota / period < host');
expect(PROBE_COMPUTER_INFO).toContain('printf "%.6g\n", quota / period');

Those break when someone reformats the awk without changing what it does, and they pass if the logic is wrong but the substring survives. The integration cases underneath them already prove the behaviour against synthetic cgroup trees, and those are the ones with teeth — twelve layouts including the combined cpu,cpuacct controller, which I would not have thought to cover.

CONTRIBUTING.md puts it as: test the logic that would rot silently, not the shape of the implementation. I would drop the toContain assertions and keep the table. Your call — I am not going to hold the PR over it.

Gating the integration block on process.platform === 'linux' is right, and worth saying out loud: it means those cases only run on the ubuntu jobs. That is the correct trade, since the probe is Linux-only by construction.

Note for the maintainer, not for you

Your run needed manual approval again. GitHub gates workflow runs per-PR for outside contributors until the setting is changed, so this will keep happening and is not something you can avoid from your side — I will raise it separately.

@Hotragn
Hotragn merged commit 6e8a017 into Hotragn:main Sep 21, 2026
10 of 12 checks passed
@Audgui-Byte

Copy link
Copy Markdown
Contributor Author

Thanks for the real-container verification and for merging this. Your point about the source-text assertions is fair: they freeze the awk spelling without proving its behavior. I have removed the two new implementation-string tests locally while retaining the 12 behavioral cases and their expected values. The focused suite passes (6 passed, 12 Linux-only skips on Windows), and the separate shell-fixture runner passes all 24 runs. That cleanup is local only; it is not part of the merged commits.

The lesson I am taking into future contributions is to test the result a caller depends on, and to give duplicated rules a behavioral check against explicit expected values. Thanks for making the review concrete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: core Touches packages/core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

computer_info reports the host's CPU count and no memory at all, when both limits are husk's own

2 participants