Skip to content

animus-cli: bound plugin/daemon tokio worker threads to cgroup CPU quota (not host cpu count) - #342

Merged
animus-launchapp-gitprovider[bot] merged 1 commit into
mainfrom
animus/TASK-628
Jul 30, 2026
Merged

animus-cli: bound plugin/daemon tokio worker threads to cgroup CPU quota (not host cpu count)#342
animus-launchapp-gitprovider[bot] merged 1 commit into
mainfrom
animus/TASK-628

Conversation

@animus-launchapp-gitprovider

Copy link
Copy Markdown
Contributor

Automated change for TASK-628 via the Animus coding workflow (deterministic git on a shared ephemeral node).

@vercel

vercel Bot commented Jul 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
animus-docs Ready Ready Preview Jul 30, 2026 10:39pm

Request Review

@Shooksie Shooksie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cannot merge as-is: rustfmt is failing, and the branch predates the current rc.33/runtime refactor lineage. Rebase onto current main, keep the cgroup-quota thread calculation isolated from unrelated runtime changes, rerun workspace fmt/clippy/check/tests, and provide Linux cgroup v1/v2 plus unlimited/malformed quota coverage on the rebased head.

@animus-launchapp-gitprovider animus-launchapp-gitprovider Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: CHANGES REQUESTED

  1. crates/orchestrator-plugin-host/src/cgroup_threads.rs (parse_cgroup_v1) — the new head contains the unformatted declaration pub(crate) fn parse_cgroup_v1(quota_str: &str, period_str: &str) -> Option<usize> {, and the required rustfmt check is currently failing on head 6c339dc76a78abf52794c792d031d606233e0a25. REQUIRED CHANGE: run workspace formatting and commit every formatting change produced for this PR (cargo fmt --all); do not merge while the required rustfmt job is red. Verification: run cargo fmt --all -- --check and confirm it exits 0, then push and confirm the GitHub rustfmt, cargo-check (workspace), clippy, and Rust dependency guardrail checks all complete successfully.

The implementation is otherwise focused on TASK-628: it introduces one shared cgroup v2/v1 fallback helper, preserves the explicit TOKIO_WORKER_THREADS override, applies the value to the in-repo runtime entry points and child-process seams, and includes useful quota/unlimited/floor parsing coverage.

@animus-launchapp-gitprovider animus-launchapp-gitprovider Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: CHANGES REQUESTED

  1. crates/orchestrator-plugin-host/src/cgroup_threads.rs (parse_cgroup_v1) — the synchronized head still contains pub(crate) fn parse_cgroup_v1(quota_str: &str, period_str: &str) -> Option<usize> {, and both required rustfmt check runs fail on head 1e39b2f8a7bd3602eb304da818aa4cbe8a59be33. REQUIRED CHANGE: run cargo fmt --all from the workspace root and commit the formatter's multiline rewrite (plus every other formatter-generated change) to animus/TASK-628; do not hand-edit only one line or merge a red head. Verification: run cargo fmt --all -- --check and confirm exit code 0 locally, then push and confirm the GitHub rustfmt check completes successfully; also wait for the currently running workspace cargo-check, clippy, smoke-help, and Rust dependency guardrail jobs to finish successfully.

The implementation remains focused and technically well structured: it centralizes cgroup v2/v1 quota parsing, preserves the operator override, applies the computed limit at the relevant in-repo runtime and child-process seams, and includes useful malformed/unlimited/floor coverage.

@animus-launchapp-gitprovider animus-launchapp-gitprovider Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: CHANGES REQUESTED

  1. crates/orchestrator-plugin-host/src/cgroup_threads.rs (parse_cgroup_v1) — head 4ffab8f136a8acf111174e049a964d9c0e1ad27d still contains the rustfmt-rejected declaration pub(crate) fn parse_cgroup_v1(quota_str: &str, period_str: &str) -> Option<usize> {, and both rustfmt runs on this synchronized head completed with failure. REQUIRED CHANGE: run cargo fmt --all from the workspace root and commit every formatter-generated rewrite to animus/TASK-628, including rustfmt’s multiline signature rewrite for parse_cgroup_v1; do not hand-edit only the reported line. Verification: run cargo fmt --all -- --check and require exit code 0 before pushing, then confirm the GitHub rustfmt check is successful and that workspace cargo-check, clippy, smoke-help, and the Rust dependency guardrail all finish successfully on the same head SHA.

The implementation itself remains focused and well aligned with TASK-628: it centralizes cgroup v2/v1 quota parsing, preserves the explicit operator override, applies the computed limit at the relevant in-repo runtime and child-process seams, and includes useful quota, unlimited, malformed, and floor coverage.

@animus-launchapp-gitprovider animus-launchapp-gitprovider Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: CHANGES REQUESTED

  1. crates/orchestrator-plugin-host/src/cgroup_threads.rs:78pub(crate) fn parse_cgroup_v1(quota_str: &str, period_str: &str) -> Option<usize> { remains in the unformatted one-line form, and the rustfmt check is failing on head 6b444eb7e678e779a3f0c91032bbc169dc6d93f9. REQUIRED CHANGE: run cargo fmt --all from the workspace root and commit every formatter-generated rewrite on the existing animus/TASK-628 branch; do not hand-edit only this signature. Verify with cargo fmt --all -- --check, which must exit 0, and confirm the GitHub rustfmt check succeeds on the resulting pushed SHA.

The cgroup-v2/v1 fallback logic, explicit TOKIO_WORKER_THREADS override, runtime-builder integration, and focused regression tests otherwise align well with TASK-628.

@animus-launchapp-gitprovider animus-launchapp-gitprovider Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: CHANGES REQUESTED

  1. crates/orchestrator-plugin-host/src/cgroup_threads.rs:78 — head 03cf7507dc95db6edbb1db51d9d8871a6b6f4bcd still contains the rustfmt-rejected one-line declaration pub(crate) fn parse_cgroup_v1(quota_str: &str, period_str: &str) -> Option<usize> {, and both rustfmt check runs on this synchronized head completed with failure. REQUIRED CHANGE: from the workspace root, run cargo fmt --all and commit every formatter-generated rewrite to the existing animus/TASK-628 branch, including rustfmt's multiline rewrite of this signature; do not hand-edit only the reported line. Verify with cargo fmt --all -- --check and require exit code 0 before pushing, then confirm the GitHub rustfmt check succeeds on the resulting head SHA and that the remaining workspace cargo-check, clippy, smoke-help, and Rust dependency guardrail jobs finish successfully.

The implementation itself remains focused and well aligned with TASK-628: it centralizes cgroup v2/v1 quota parsing, preserves the explicit TOKIO_WORKER_THREADS override, applies the computed limit at the relevant in-workspace runtime and child-process seams, and includes useful quota, unlimited, malformed, and floor coverage.

@animus-launchapp-gitprovider animus-launchapp-gitprovider Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: CHANGES REQUESTED

  1. crates/animus-runtime-utils/src/cgroup_threads.rs:78 — the current head 2db8247072a89b3c7be1edbe1b8d868502a9a99b still contains the rustfmt-rejected one-line declaration pub(crate) fn parse_cgroup_v1(quota_str: &str, period_str: &str) -> Option<usize> {, and the required rustfmt check completed with failure. REQUIRED CHANGE: run cargo fmt --all from the workspace root and commit every formatter-generated rewrite on animus/TASK-628, including rustfmt's multiline rewrite of this signature; do not hand-edit only this line. Verify by running cargo fmt --all -- --check and requiring exit code 0 before pushing, then confirm GitHub rustfmt succeeds on the resulting head SHA. Also wait for cargo-check, clippy, and the dependency guardrail to complete successfully; the currently failing docs-sync check must be green before merge.

The implementation remains focused and technically well aligned with TASK-628: the shared helper preserves the explicit environment override, handles cgroup v2 and v1 quotas with safe fallbacks, is wired into the relevant in-workspace runtime seams, and has useful malformed, unlimited, fractional-quota, and floor coverage.

@animus-launchapp-gitprovider animus-launchapp-gitprovider Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: CHANGES REQUESTED

  1. crates/animus-runtime-utils/src/cgroup_threads.rs:78pub(crate) fn parse_cgroup_v1(quota_str: &str, period_str: &str) -> Option<usize> { is still not in the canonical rustfmt layout, and the rustfmt check fails on head 887e9351e1fcb2fdcb7fe0c4244c9eb4b09bc17e. REQUIRED CHANGE: run cargo fmt --all on this exact head and commit every formatter-generated rewrite; in particular, let rustfmt split this function signature into its multiline form rather than preserving the current single line. Verify with cargo fmt --all -- --check; it must exit 0, and the GitHub rustfmt check must conclude success on the pushed SHA.

  2. Cargo.toml:2 / docs/architecture/full-system-architecture.md (Workspace Inventory) — the workspace now contains 11 members after adding "crates/animus-runtime-utils", while the documentation still says ``Cargo.toml currently declares 10 workspace members. The same stale count remains in `docs/architecture/crate-map.md`, and `scripts/check-doc-sync.sh` requires the count to agree across all guarded documentation, which is why `docs-sync` fails on this head. REQUIRED CHANGE: update the workspace count from 10 to 11 in `docs/architecture/full-system-architecture.md`, `docs/architecture/crate-map.md`, `docs/contributing/development.md`, `docs/design/acp-integration.md`, and `docs/architecture/index.md`; also add `animus-runtime-utils` to the appropriate workspace inventory/crate table so the new shared helper is documented rather than only changing the number. Verify with `bash scripts/check-doc-sync.sh`; it must exit 0 and print `CLI command tree and MCP tool reference are in sync.`, and the GitHub `docs-sync` check must conclude success.

The core approach is otherwise well aligned with TASK-628: the helper is dependency-light, honors TOKIO_WORKER_THREADS first, handles cgroup v2/v1 quota parsing with tests, and keeps the implementation scoped to this repository. Cargo-check, clippy, and the dependency guardrail are still running; all required checks must be green on the same head before merge.

@animus-launchapp-gitprovider animus-launchapp-gitprovider Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: CHANGES REQUESTED

  1. crates/animus-runtime-utils/src/cgroup_threads.rs:78pub(crate) fn parse_cgroup_v1(quota_str: &str, period_str: &str) -> Option<usize> { is not in the repository's formatter-approved layout, and the rustfmt check is failing on head 887e9351e1fcb2fdcb7fe0c4244c9eb4b09bc17e. REQUIRED CHANGE: run cargo fmt --all on this exact head and commit every formatter-generated rewrite, including the multiline formatting of parse_cgroup_v1. Verify with cargo fmt --all -- --check; it must exit 0, and the GitHub rustfmt check must complete successfully on the pushed SHA.

  2. docs/architecture/full-system-architecture.md:29, docs/architecture/crate-map.md:3, docs/contributing/development.md:32, docs/design/acp-integration.md (Current Animus Architecture), and docs/architecture/index.md:3 — the new crate makes this an 11-member workspace, but these files still say 10 workspace members, 10 crates, or 10 current workspace members; the workspace inventories also omit animus-runtime-utils. This exact drift is reflected by the failing docs-sync check. REQUIRED CHANGE: change every stale workspace count from 10 to 11 and add animus-runtime-utils to the applicable workspace inventory/crate table (and development tree), describing it as the dependency-light cgroup/runtime sizing helper introduced by this PR. Verify with bash scripts/check-doc-sync.sh; it must exit 0, and the GitHub docs-sync check must complete successfully on the same pushed SHA.

The implementation is otherwise focused and promising: it centralizes cgroup v2/v1 quota parsing in a dependency-light crate, preserves TOKIO_WORKER_THREADS precedence, includes useful parser edge-case tests, and the workspace cargo-check, clippy, smoke, and dependency-guardrail checks are already green. Because rustfmt and docs-sync remain red, this head must not be merged yet.

@Shooksie

Copy link
Copy Markdown
Contributor

Rework complete on exact head fa523bfa: replaced the stale nine-commit/conflicting lineage with one commit on current main (including merged #334); resolved current rc.36 protocol dependencies; removed the new num_cpus dependency in favor of std::thread::available_parallelism; updated all workspace inventory docs from 9 to 10 members; and kept the change scoped to cgroup-aware worker sizing and propagation. Local evidence: cargo fmt --all -- --check; bash scripts/check-doc-sync.sh; cargo check --workspace --all-targets --offline; cargo test -p animus-runtime-utils --offline (20/20); plugin-host manifest propagation regression; CLI manifest-probe propagation regression; cargo metadata --locked --offline. Fresh GitHub checks are authoritative.

@animus-launchapp-gitprovider animus-launchapp-gitprovider Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: APPROVE

The implementation is focused and complete for TASK-628. It introduces a dependency-light helper that honors TOKIO_WORKER_THREADS, then reads cgroup v2 and v1 quotas with a safe system fallback and a floor of one. The daemon/CLI runtime entry points and clean-environment plugin/workflow-runner launch seams consistently apply the computed bound, with parser and propagation regression coverage. Workspace inventory documentation and Cargo metadata are updated without unrelated code churn.

All checks on head fa523bfa1f8b1d9fca760cbee64bd9253088d126 are green: rustfmt, clippy, workspace cargo-check, docs-sync, Rust-only dependency guardrail, smoke-help, and Vercel. The branch is mergeable against main.

What is genuinely good: the shared helper avoids dependency cycles, preserves the operator override, handles malformed/unlimited quotas conservatively, and tests both quota arithmetic and the env propagation paths that previously lost the bound after env_clear().

@Shooksie
Shooksie dismissed their stale review July 30, 2026 22:42

Dismissed after exact requested rework: rebased to current main, reduced to one focused commit, formatter/docs/workspace/clippy/smoke/guardrail checks all green, and cgroup v1/v2 unlimited/malformed coverage passes.

@animus-launchapp-gitprovider
animus-launchapp-gitprovider Bot merged commit 7d9f6be into main Jul 30, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant