test: drop source-shape tests from #2851 to unbreak main - #2863
Conversation
#2851 merged just after #2829 enforced maxSourceShapeCases=0. The 11 toMatch assertions in test/repro-2681-group-writable.test.ts violated the new policy, breaking main on the source-shape budget hook. Behavioral coverage of the group-writable contract is provided by: - build-sandbox-images (Dockerfile syntax + chmod application) - test-e2e-sandbox (sandbox lifecycle including writes) - test-e2e-gateway-isolation (gateway-as-different-UID writes) A proper behavior-based regression test for #2681 can land as a follow-up if the existing e2e coverage proves insufficient.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRemoved a regression test file that validated group-writable permission behavior; added a no-op Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
The runPreGatewaySetup helper stubs every helper the script invokes; #2851 added a normalize_mutable_config_perms call but no stub, so the test bash run hits an undefined function under set -euo pipefail and exits 127.
Summary
Hotfix. Main is red on the
source-shape-test-budgethook because #2851 merged at 22:23 UTC just after #2829 enforcedmaxSourceShapeCases=0at 21:30 UTC. My PR's CI ran under the old regime; the merge landed under the new policy.This drops the 11 source-shape (toMatch) assertions in
test/repro-2681-group-writable.test.ts. They violated the new policy.Why deletion is the right hotfix
The structural-invariant assertions were redundant with behavioral CI we already have:
build-sandbox-images— proves the Dockerfile + Dockerfile.base + chmod changes BUILD correctlytest-e2e-sandbox— sandbox lifecycle works end-to-end (group-writable dir gets created)test-e2e-gateway-isolation— proves gateway-as-different-UID can perform mutable config writesIf a future change reverts the chmod g+w or the gateway group membership,
test-e2e-gateway-isolationwould catch it because gateway writes to OpenClaw config would fail.Failing job (before hotfix)
https://github.com/NVIDIA/NemoClaw/actions/runs/25235761538/job/74001520150
After hotfix
Follow-up
A behavior-based regression test for #2681 (e.g. inside test-e2e-gateway-isolation that explicitly attempts a write from gateway UID) can land as a separate PR if the existing e2e coverage proves insufficient.
Type of Change
AI Disclosure
Summary by CodeRabbit