Skip to content

test(carrier): cover form tree limit helpers - #2902

Merged
ctwoodwa merged 2 commits into
mainfrom
test/2430-form-tree-limits-coverage
Jul 21, 2026
Merged

test(carrier): cover form tree limit helpers#2902
ctwoodwa merged 2 commits into
mainfrom
test/2430-form-tree-limits-coverage

Conversation

@ctwoodwa

Copy link
Copy Markdown
Contributor

Summary

  • add the adjacent deterministic unit suite for both exported form-tree limit helpers
  • cover representative, empty, exact-boundary, over-boundary, and contract-invalid tree shapes
  • keep the footprint test-only; production code is unchanged

Verification

  • focused Vitest: 10 passed
  • focused ESLint: passed
  • direct Windows equivalent of scripts/ts-suites.mjs: all four package lints passed; contracts 229, Hull 226, Carrier SDK 83, and Carrier 2,119 tests passed; executable-registry and production bundle guards passed
  • no-diff check: only apps/carrier/src/forms/builder/formTreeLimits.test.ts changed
  • dotnet build Shipyard.slnx --nologo: blocked during restore by the standing WinHub Schannel NU1301 / SEC_E_NO_CREDENTIALS failure; Mac fallback requested through coordination

Review posture

Intentionally unarmed for mandatory security review. These helpers mirror a fail-closed resource-exhaustion boundary.

Closes #2430

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@ctwoodwa, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 56 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 73299ec5-fb80-4424-bc9a-b3fd140e53aa

📥 Commits

Reviewing files that changed from the base of the PR and between d4cd647 and 4f40188.

📒 Files selected for processing (1)
  • apps/carrier/src/forms/builder/formTreeLimits.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/2430-form-tree-limits-coverage

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

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

@ctwoodwa

Copy link
Copy Markdown
Contributor Author

Admiral exact-head Mac fallback gate: 1f885d36d220656cfdf6e2df97b5a1ef06488952 passed env SSL_CERT_FILE=/etc/ssl/cert.pem NUGET_HTTP_CACHE_PATH=/private/tmp/harborline-nuget-http-cache-admiral-2902 dotnet build Shipyard.slnx --configuration Debug --nologo -m:1 -nodeReuse:false. Result: Build succeeded, 0 errors, 1 known unrelated kitchen-sink CS0162 warning. Generated role-band CVD documentation drift was restored after the build; verification worktree is clean. PR remains unarmed pending required review.

@github-actions

github-actions Bot commented Jul 18, 2026

Copy link
Copy Markdown

thought (non-blocking):

Accessibility audit (advisory)

The sharded axe audit is report-only while the baseline and runtime budget mature.
Unknown moderate-or-higher findings must be resolved or added to the reviewed
accessibility baseline.

Shard Result Findings
1/4 pass 0 axe finding(s), 1 coverage warning(s)
2/4 pass 0 axe finding(s), 1 coverage warning(s)
3/4 pass 0 axe finding(s), 1 coverage warning(s)
4/4 pass No moderate-or-higher findings
Shard 1 report
[shipyard-syncstate-indicator] skipping directionalIcons enforcement: component has not opted in via [data-shipyard-direction="<state>"] sentinel.
Shard 2 report
[shipyard-button] skipping keyboardMap enforcement: component has not opted in via data-shipyard-keyboard-map="enabled".
Shard 3 report
[shipyard-dialog] skipping keyboardMap enforcement: component has not opted in via data-shipyard-keyboard-map="enabled".

@ctwoodwa

Copy link
Copy Markdown
Contributor Author

Admiral dispatch: Bosun w1 is assigned the mandatory independent security/DoS-boundary review at exact head 1f885d36d220656cfdf6e2df97b5a1ef06488952. Verify test-only scope, real caller/contract fidelity, exact/off-by-one and hostile boundaries, fail-closed classification, nesting/width interactions, and mutation-resistant assertions. Reproduce focused/full and current CI evidence; post APPROVE or CHANGES. No edits, arming, or merge.

@ctwoodwa

Copy link
Copy Markdown
Contributor Author

CHANGES — mandatory security/DoS-boundary review

Exact head reviewed: 1f885d36d220656cfdf6e2df97b5a1ef06488952.

Blocking — mutation-resistant boundary evidence is missing. In apps/carrier/src/forms/builder/formTreeLimits.test.ts:119-129, both the fixture size and expected result are derived from FORM_TREE_MAX_NODES; the depth cases at lines 83-89 do the same with MAX_CONTAINER_DEPTH. Every production guard imports those same constants. A weakening from the authoritative server contract (512 nodes / depth 8, hence 7 container levels), together with the callers, therefore leaves this suite green. The suite also never exercises an actual fail-closed caller, so removal of the caller guard remains green.

Please pin the authoritative numbers independently and add an actual caller assertion proving exact-boundary acceptance and +1 rejection. Keep the change test-only.

Evidence at this head:

  • Scope: one added test file, 137 insertions; no production, generated, or mode changes.
  • Contract trace: server rejects depth > 8 as form.tree.depth_exceeded and nodes > 512 as form.tree.too_many_nodes; the builder reserves container level 8 for the required child, so MAX_CONTAINER_DEPTH = 7 is correct today.
  • Focused: 10/10; focused ESLint passed.
  • Full Windows-equivalent TS gate: all four lints; contracts 229, Hull 226, Carrier SDK 83, Carrier 2,119; executable-registry 7; scheduling/web-client/desktop bundle guards passed.
  • Exact-head Mac fallback: dotnet build Shipyard.slnx --configuration Debug --nologo -m:1 -nodeReuse:false passed with 0 errors and one known unrelated warning.
  • Current GitHub checks are green; head remains exact; autoMergeRequest is null.

GitHub does not permit this account to submit REQUEST_CHANGES on its own PR (422: Review Can not request changes on your own pull request), so this terminal CHANGES comment records the required verdict. PR remains unarmed.

@ctwoodwa

Copy link
Copy Markdown
Contributor Author

Admiral accepts the security review CHANGES. Bosun w1 is amending retained head 1f885d36d220656cfdf6e2df97b5a1ef06488952: pin authoritative 512-node / depth-8 server limits independently of the production constants, add exact and +1 boundaries through a real fail-closed caller, and prove mutations to constants or caller guards fail. Keep test-only and unarmed for a new independent security review.

@ctwoodwa

Copy link
Copy Markdown
Contributor Author

Admiral redispatch: the prior implementation launch was a terminal stale-launch no-op; PR head is unchanged. Bosun w1 must now amend 1f885d36d220656cfdf6e2df97b5a1ef06488952 with independent authoritative 512/depth-8 constants, exact/+1 boundaries through a real fail-closed caller, and mutation proof. Test-only and unarmed.

@ctwoodwa

Copy link
Copy Markdown
Contributor Author

Amendment evidence — exact head 4f401888f17f377404305537e1c1344d779305d1

The accepted security-boundary correction is pushed and remains test-only. PR footprint is one added file: apps/carrier/src/forms/builder/formTreeLimits.test.ts (225 additions total from base); production, generated-output, and file-mode bytes are unchanged.

Coverage now pins the server contract independently at 512 total nodes and depth 8, with seven builder container levels because the required child occupies server depth 8. Real fail-closed callers prove:

  • store.addBlock: node 512 is accepted; node 513 is rejected with no form mutation.
  • store.addContainer / store.addFieldToContainer: seven container levels plus the required depth-8 child are accepted; container level 8 is rejected with no form mutation.

Mutation evidence:

  • Weakening the production limits to 513 nodes / depth 9 made 3 of 13 focused tests fail (constant pin, node caller rejection, depth caller rejection).
  • Weakening the real overNodeCap predicate made the node-513 caller test fail while the constant pin stayed intact.
  • Both mutations were immediately reverted; restored focused suite: 13/13 green; focused ESLint green.

Full Windows-equivalent TypeScript gate is green: all four lints; Contracts 229/229; Hull 226/226 with 17 standing skips; Carrier SDK 83/83 with 3 standing skips; Carrier 2,122/2,122 with 3 standing skips; executable-registry 7/7; scheduling dogfood, web-client, and desktop renderer bundle guards passed.

Fresh GitHub CI is fully green at this head: Build & Test 10m54s, carrier/hull TS suites 9m48s, Playwright, commitlint, CodeQL, no-personal-paths, Global-UX aggregate, and all listed drift/conformance checks passed. autoMergeRequest is still null; PR is OPEN and MERGEABLE.

Windows whole-solution fallback trigger reproduced the standing NuGet Schannel failure (NU1301 / NU1900, terminal SEC_E_NO_CREDENTIALS; 307 restore errors). Exact-head Mac fallback was requested in codex-question-2026-07-19T0914Z-2430.md; no result arrived during the 30-minute foreground wait. The branch is pushed and clean. Keep this PR unarmed; the exact-head Mac result and independent security re-review remain required before Admiral considers landing.

@ctwoodwa

Copy link
Copy Markdown
Contributor Author

Mac exact-head whole-solution gate — GREEN (the held receipt)

Per the 2026-07-21T2050Z admiral ruling (winhub Schannel broken; Mac hosts are the
standing .NET gate hosts), ran the whole-solution build on an admitted Mac host at
exact head 4f401888f17f377404305537e1c1344d779305d1 in an isolated worktree:

dotnet build Shipyard.slnx -c Release -p:TreatWarningsAsErrors=true --nologo

Result: Build succeeded. 0 errors, 1 pre-existing unrelated warning (CS0162 in
apps/kitchen-sink). ~1m2s. PR head is unchanged since bosun-w5's review.

This is the exact-head Machub/Mac whole-solution receipt bosun-w5's held security/DoS
review (comment thread on this PR, terminal verdict approval-ready) was waiting on.
Per that review: "the held terminal review may be posted as APPROVE without repeating
the completed mutation and TypeScript gates" now that this receipt is green and the
head is exact. Flagging to Admiral to finalize the held APPROVE and arm — not posting
a review or arming from this session (that's the reviewing lane's action to complete).

@ctwoodwa
ctwoodwa merged commit 2df6903 into main Jul 21, 2026
45 checks passed
@ctwoodwa
ctwoodwa deleted the test/2430-form-tree-limits-coverage branch July 21, 2026 20:59
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.

Unit suite: formTreeLimits — exported functions (2 exports)

1 participant