test/rebuild-policy-presets.test.ts (1)

133-161: Tests verify expected fallback pattern but don't exercise actual restore code.

These tests document the expected || [] fallback behavior used in nemoclaw.ts:1886, which is valuable for regression prevention. However, they test JavaScript semantics rather than the actual sandboxRebuild function.

Consider adding an integration test that mocks policies.applyPreset() and verifies it's called with the expected preset names during rebuild. This would provide stronger confidence in the restore path.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/rebuild-policy-presets.test.ts` around lines 133 - 161, Add an
integration-style test that invokes sandboxRebuild (the rebuild entrypoint) with
a backup/manifest that contains policyPresets = ["telegram","npm"], mock
policies.applyPreset to a spy/stub, run sandboxRebuild, and assert
policies.applyPreset was called twice with "telegram" and "npm" (and not called
when manifest.policyPresets is undefined/null or empty); ensure the test sets up
the same || [] fallback scenario and restores the mock after the test so other
tests are unaffected.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

test/e2e/test-rebuild-openclaw.sh (1)

434-439: ⚠️ Potential issue | 🟡 Minor

Manifest preset assertion still isn’t exact-set validation.

This still checks containment only; extra or malformed preset names can pass. The assertion should compare actual vs expected sets and report missing/extra entries explicitly.

Use exact-set comparison with clear mismatch output

-  if echo "${MANIFEST_PRESETS}" | grep -q "npm" \
-    && echo "${MANIFEST_PRESETS}" | grep -q "pypi"; then
-    pass "Backup manifest contains policyPresets: ${MANIFEST_PRESETS}"
-  else
-    fail "Backup manifest missing expected policyPresets (npm,pypi): got '${MANIFEST_PRESETS}' — issue `#1952`"
-  fi
+  MANIFEST_SET_CHECK=$(python3 -c "
+expected = {'npm', 'pypi'}
+raw = '${MANIFEST_PRESETS}'
+actual = {x for x in raw.split(',') if x and x not in {'NONE', 'error'} and not x.startswith('ERROR:')}
+missing = sorted(expected - actual)
+extra = sorted(actual - expected)
+print('OK' if not missing and not extra else f'missing={missing}, extra={extra}')
+")
+  if [ \"${MANIFEST_SET_CHECK}\" = \"OK\" ]; then
+    pass \"Backup manifest contains expected policyPresets: ${MANIFEST_PRESETS}\"
+  else
+    fail \"Backup manifest policyPresets mismatch (${MANIFEST_SET_CHECK}): got '${MANIFEST_PRESETS}' — issue `#1952`\"
+  fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-rebuild-openclaw.sh` around lines 434 - 439, Replace the
containment checks on MANIFEST_PRESETS with an exact-set comparison: parse
MANIFEST_PRESETS into a normalized sorted list (split on commas/whitespace),
build the expected set ["npm","pypi"], compare both sets for equality, and call
pass when equal; on mismatch call fail and include explicit missing entries
(expected but not present) and extra entries (present but not expected). Update
the conditional around MANIFEST_PRESETS and the pass/fail branches (the logic
currently using grep -q) to perform this set diff so
test/e2e/test-rebuild-openclaw.sh reports precise missing/extra preset names.

test/e2e/test-rebuild-openclaw.sh (1)

396-405: Registry preset survival check is substring-based and can false-pass.

Line 396 and Line 401 grep for "npm"/"pypi" in a CSV string. Values like npm-mirror would pass. Parse as a set and compare exact expected values.

Tighten to an order-independent exact-set assertion

-if echo "${POST_REBUILD_PRESETS}" | grep -q "npm"; then
-  pass "npm preset survived rebuild (in registry)"
-else
-  fail "npm preset LOST after rebuild — issue `#1952`"
-fi
-if echo "${POST_REBUILD_PRESETS}" | grep -q "pypi"; then
-  pass "pypi preset survived rebuild (in registry)"
-else
-  fail "pypi preset LOST after rebuild — issue `#1952`"
-fi
+REGISTRY_SET_CHECK=$(python3 -c "
+expected = {'npm', 'pypi'}
+actual = {x for x in '${POST_REBUILD_PRESETS}'.split(',') if x and x != 'error'}
+missing = sorted(expected - actual)
+extra = sorted(actual - expected)
+print('OK' if not missing and not extra else f'missing={missing}, extra={extra}')
+")
+if [ "${REGISTRY_SET_CHECK}" = "OK" ]; then
+  pass "Registry presets survived rebuild exactly: ${POST_REBUILD_PRESETS}"
+else
+  fail "Registry presets mismatch after rebuild (${REGISTRY_SET_CHECK}) — issue `#1952`"
+fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-rebuild-openclaw.sh` around lines 396 - 405, The current checks
use substring grep on POST_REBUILD_PRESETS which can false-pass (e.g.
"npm-mirror"); change the assertion to parse POST_REBUILD_PRESETS as a CSV, trim
each entry, and perform an order-independent exact membership test against the
expected set {"npm","pypi"} (e.g. split on ',', trim whitespace, sort/uniq
entries and compare to the sorted expected list or check each exact element
exists in the parsed array). Update the checks around POST_REBUILD_PRESETS (the
grep tests that pass/fail "npm" and "pypi") to use this exact-set comparison
instead of simple grep.