fix(codex): validate subagent limit boundaries before writes

[luvs01]

Summary

• Display zero children when global V2 stores a total thread limit of one, in both the Models selector and folded summary. Entering one child then sends a total of two instead of being treated as an unchanged value. V1 and hybrid mode retain child-count units.
• Validate safe integers at both request entry points. The GUI rejects unsafe child counts and unsafe values after adding the V2 root slot before sending a PUT. The API returns 400 for unsafe limits before feature toggles, convergence or configuration writes.
• Document the display and validation boundaries. This follows the existing upstream discussions about root-only display and unsafe limit validation.

Verification

GitHub's update-branch operation merged current dev into the PR branch. Signed publication HEAD: 21db48ca329b97f18f3e3e667c870c76bec11d14; first parent: 211a5ea51ef0874444b4d2a5c380abea87acc782; second parent (dev): 8640c2f30edbd2fbb6947ed5c25e2793f5ae72a6; tree: e9c58ed178ba2d6bfa1df0360adc495e54d04e96. GitHub signature verification is valid, author luvs01, committer GitHub (web-flow). The server tree exactly matches the locally validated merge candidate. Its 7-file, 15,802-byte delta against dev remains byte-identical to the original PR patch, SHA256 b6cf0c8df04516712df60433fec9ad1fdea2cefd6ed9a03f15460ccbb849b7cd; all seven changed file blobs are unchanged.

Fresh local validation of that merge tree on 2026-10-18:

• Repository-pinned Bun 1.4.3, root: bun test ./tests/codex-integration/codex-agent-thread-limit.test.ts --timeout 30000 — 19 passed, 0 failed, 120 assertions, 11.69 seconds. Repository fixtures isolate homes and stub native hooks; this is not a live capacity test.
• Pinned Bun 1.4.3, gui/: bun test ./tests/models-subagent-limit.test.tsx --timeout 30000 — 20 passed, 0 failed, 71 assertions, 4.07 seconds, using fixture DOM/fetch. These focused scopes are separate and are not added to historical suite counts.
• Node 24.18.0, root TypeScript 7.0.2: direct node node_modules/typescript/bin/tsc --noEmit — PASS. GUI TypeScript 6.0.3 application and config checks also PASS, using candidate-local build-info files.
• GUI production bundle via direct Node/Vite with --configLoader native, and direct Node/oxlint — PASS. The existing large-chunk warning remains; this is not a warning-free claim.
• Pinned Bun structure SSOT, privacy scan and file-size ratchet — PASS. Latest-dev-relative staged diff check — PASS.
• Documentation through the unmodified Astro API under pinned Bun — PASS, 577 pages / 81,460 internal links. Initial Node execution failed with ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING; the first Bun caller used an invalid URL-valued cacheDir. Correcting that task-local argument to a string produced the passing build; no repository/vendor helper was patched. No dependencies were installed for these checks. Separate redundant runtime preparation was stopped and cleaned before validation used the existing pinned Bun; preparation failures are not passing test results.

Exact-head Cross-platform CI run 39097680287 on 21db48ca329b97f18f3e3e667c870c76bec11d14 completed successfully at 2026-10-18 04:46:57 UTC: 15 successful jobs, 8 skipped, 0 failed, 0 cancelled, all 23 jobs completed. Skipped platform/native jobs are not passing coverage. New-head React Doctor, Catalog async contracts, target-branch and label checks also succeeded. PR hygiene run 39097678817 succeeded; the separate body-edit-triggered hygiene run 39097719229 was cancelled, not passed. These results do not claim every merge prerequisite or approval is complete.

CodeRabbit's existing review comment covers the original head 211a5ea51ef0874444b4d2a5c380abea87acc782, not a new full review of the merge head. It reports no actionable review comments or identified merge-blocking issue, while its separate docstring coverage warning remains: 16.67% against an 80% threshold, across 6 functions. That generic documentation advisory is treated as nonblocking for author review readiness; it is not hidden or reported as passed. A maintainer may request additional docstrings. The seven changed code/document blobs remain unchanged; current CodeRabbit commit status is success, but this does not mean all advisory checks fully passed.

Original-head Cross-platform CI run 39041505444, with 15 successful and 8 skipped jobs, remains historical evidence only for 211a5ea51ef0874444b4d2a5c380abea87acc782. The prior full GUI run (3,441 passed, 351 files, 33,191 assertions) also belongs to that historical head and was not rerun for the refreshed tree. Historical target enforcement did not assert exact head equality and is not proof that all current merge checks pass.

The full root suite, test:changed and full GUI suite were NOT RUN locally for this refreshed tree under the focused/shared-resource scope. Live native effective capacity, production configuration and host reload remain UNVERIFIED. Maintainer/code-owner approval and explicit security review remain pending before merge. Author validation is complete and this PR is ready for review with the documentation advisory disclosed above. Maintainer/code-owner approval and explicit security review remain separate merge prerequisites; no approval, merge, release or deployment is claimed.

Historical screenshot fixture

The GUI blobs are unchanged, so the existing synthetic-fixture capture below is retained as historical evidence; no fresh screenshot or live behavior validation is claimed.

• Headless Models capture used synthetic API fixtures: initial selector 0 with no PUT; custom input 1 with no PUT; after apply, selector 1 and PUT {maxConcurrentThreadsPerSession:2}. PNG: 2100 × 1020, 188,311 bytes, SHA256 6d7a37c5584ada1f376c4259f4c6b802d2122b2e7fec6e588b4db2cbed47d18f.

The screenshot is on the separate pr-assets branch, pinned to its asset commit. Its downloaded bytes match the PNG SHA256 above; no image is included in the code commit.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.
• The signed publication HEAD/tree and exact-head hosted checks are recorded.
• The required screenshot is attached through an immutable asset URL.
• Maintainer/code-owner review is complete.
• My PR is ready for review.

Summary by CodeRabbit

• Bug Fixes
  • V2 sub-agent limits now display as the number of child agents, excluding the root agent. Saving a child limit sends the corresponding total thread limit.
  • Invalid or unsafe limits are rejected before settings are changed. The largest safely supported limit remains accepted, including when V2 is disabled or hybrid mode is used.
• Documentation
  • Clarified how child limits relate to V2 thread limits, including root-only settings and hybrid mode.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 6afe1131-d186-4bfe-b954-a220c3573e0f

📥 Commits

Reviewing files that changed from the base of the PR and between 2a10a2b and 211a5ea.

📒 Files selected for processing: 7

• docs-site/src/content/docs/reference/configuration/agents.md
• gui/src/pages/Models.tsx
• gui/src/pages/models-shared.ts
• gui/tests/models-subagent-limit.test.tsx
• src/server/management/agent-settings-routes.ts
• structure/subagent-capacity.md
• tests/codex-integration/codex-agent-thread-limit.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

The Models page now displays V2 limits as child counts and converts edits to API thread-limit units. The UI and management API reject values outside safe-integer bounds. Tests and documentation cover root-slot conversion, hybrid-mode behavior, and API validation.

Changes: Thread-limit units and validation

Layer / File(s)	Summary
UI child-count conversion and validation gui/src/pages/models-shared.ts, gui/src/pages/Models.tsx, gui/tests/models-subagent-limit.test.tsx, docs-site/src/content/docs/reference/configuration/agents.md	V2 totals that include a root slot display as child counts, with a stored limit of 1 displaying as 0. The UI converts child counts to API units when required and validates both the child count and converted limit as safe integers. Tests cover root-only display, custom saves, hybrid-mode behavior, and safe-integer boundaries.
Management API safe-integer validation src/server/management/agent-settings-routes.ts, tests/codex-integration/codex-agent-thread-limit.test.ts, structure/subagent-capacity.md	The /api/v2 endpoint now requires maxConcurrentThreadsPerSession to be a safe integer of at least 1. Integration tests cover rejection before side effects and acceptance of Number.MAX_SAFE_INTEGER. The documentation describes the API limit behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 211a5

The documented limit conversion and validation changes have no identified merge-blocking issue; normal checks remain appropriate.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 5 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the boundary validation changes, including validation before GUI or configuration writes. It matches a primary objective of the pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 5 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[Ingwannu]

Scoped review at 21db48ca32: the child-count conversion now shows root-only V2 as zero and validates both the child value and converted backend value before a write; PUT uses Number.isSafeInteger before combined mutations. Keep global enabled versus hybrid/catalog-selected V2 units distinct, and keep zero display separate from the minimum new edit of one child. Current Ubuntu CI succeeded. Please retain owner GUI review and the current API/storage documentation contract in readiness; this comment covers the numeric boundary only, not an autonomous UI decision or merge approval. No real config writes were made.