Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.coderabbit.yaml:
- Around line 246-258: The .coderabbit.yaml entry for E2E hints only lists
"src/lib/deploy/**" but omits state files; add a matching path_instructions
entry that references the state implementation (e.g., "src/lib/state/sandbox.ts"
and any other state restore owners) and include the same
`state-backup-restore-e2e` instructions block so reviewers of Sandbox/restore
logic see the E2E recommendation; update the YAML to include these state paths
with the same instructions content as the deploy entry.

In `@test/e2e/docs/MIGRATION.md`:
- Around line 124-125: The entries in MIGRATION.md use emoji checklist markers
"⬜" which violates the repo's MD style; replace those emoji markers with
plain-text checkbox markers such as "[ ]" for the two lines referencing
"test-state-backup-restore.sh (378) → lifecycle/state-backup-restore/" and
"test-tunnel-lifecycle.sh (472) → lifecycle/tunnel-lifecycle/" so the lines read
with "[ ]" instead of "⬜", preserving all other text and punctuation.

In `@test/e2e/test-state-backup-restore.sh`:
- Around line 194-201: The backup and restore invocations currently mask
failures by appending "|| true" and only inspecting stdout with grep; remove the
"|| true", capture the command exit status (e.g., run backup_output=$(bash
"$REPO_ROOT/scripts/backup-workspace.sh" backup "$SANDBOX_NAME" 2>&1); rc=$?),
and if rc is non‑zero call fail with the captured output (use the same pattern
for the restore invocation), otherwise proceed to validate success text (grep -q
"Backup saved"). Also apply the same change to the corresponding restore block
(lines referencing the restore call around 276-283) so you don't rely solely on
substring matching when the process actually failed.
- Around line 186-189: The current check only fails when files_written or
memory_written are zero; change it to require full expected counts before
proceeding by comparing files_written against the expected total (5) and
memory_written against its expected total (1) and call fail (same fail
function/TC-STATE-01) if either does not equal the expected value so partial
setups (e.g., 3/5) are rejected; update the conditional that references
files_written and memory_written accordingly and keep the existing fail message
semantics.

In `@test/e2e/test-tunnel-lifecycle.sh`:
- Around line 160-205: get_cloudflared_log_path can cause the script to exit
under set -euo pipefail when the glob matches nothing; change its command
substitution to prevent non-zero exit (e.g., capture ls output with fallback:
log=$(ls -t /tmp/nemoclaw-services-*/cloudflared.log 2>/dev/null || true)) and
make the function always return 0 (echo "" and return 0 instead of returning 1)
so callers like classify_cloudflared_log and show_cloudflared_log don't
terminate the script; ensure callers treat an empty string as "no log" (they
already check -z) so behavior stays the same.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/e2e/test-state-backup-restore.sh`:
- Line 140: The test forcibly removes the onboard lock file with rm -f
"$HOME/.nemoclaw/onboard.lock" which can clobber a legitimately held lock;
delete that rm invocation from test/e2e/test-state-backup-restore.sh and instead
rely on the existing onboard stale-lock handling described in the teardown
notes, or replace it with a safe ownership check that only removes the lock if
it is owned by the current test (e.g., read the PID/owner from the onboard.lock
and compare before deleting) so you no longer unconditionally remove
onboard.lock.

---

Duplicate comments:
In `@test/e2e/test-state-backup-restore.sh`:
- Around line 193-200: The test currently masks failures by running the backup
command with "|| true" and only inspecting backup_output; instead run the backup
command without swallowing errors, capture both its stdout/stderr into
backup_output and its exit code (e.g., via bash -c or by saving $? immediately
after), log the output (backup_output) and then assert success based on the exit
code rather than just grepping for "Backup saved"; apply the same change for the
restore invocation (restore-workspace.sh / restore_output) so failures are
detected and reported with both output and a non-zero exit status.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/e2e/test-state-backup-restore.sh`:
- Around line 216-217: The marker checks use regex grep which can falsely match
metacharacters; update the grep invocations that test for
"${marker_content}_${f}" (used alongside variables backup_dir, f, marker_content
and incrementing backup_files_ok) to use fixed-string matching by replacing
"grep -q" with "grep -Fq --" so the marker is treated literally; apply the same
change to the other similar grep calls in this script (the occurrences around
the other marker checks).

In `@test/e2e/test-tunnel-lifecycle.sh`:
- Around line 213-219: The readiness probe function probe_local_dashboard
currently appends a fallback echo which causes duplicate "000000" values on
failure; change the logic to capture curl's output into code and only set
code="000" if curl actually fails (e.g., run curl, check its exit status, and
assign "000" on non-zero), so the probe returns exactly "000" on failure. Also
fix the lock-handling discrepancy: do not unconditionally unlink
~/.nemoclaw/onboard.lock inside onboard_sandbox; instead only remove the lock if
the current process owns it (check PID ownership) or defer removal to the
teardown logic that is PID-aware, keeping the contract between onboard_sandbox
and teardown consistent.

---

Outside diff comments:
In `@test/e2e/test-tunnel-lifecycle.sh`:
- Around line 415-417: The file documents that ~/.nemoclaw/onboard.lock is
PID-ownership-aware and must not be unlinked during teardown, but the function
onboard_sandbox() still deletes that lock before onboarding; remove the
pre-onboard unlink of ~/.nemoclaw/onboard.lock from onboard_sandbox() (or guard
it so it does not remove that specific file) so the lock contract is consistent
across jobs and only the sandbox-teardown logic manages stale locks.

---

Duplicate comments:
In `@test/e2e/test-state-backup-restore.sh`:
- Around line 192-199: The test is masking backup/restore failures by using "||
true" when running the scripts and only inspecting output text; update the
commands that invoke backup-workspace.sh (the assignment to backup_output) and
the corresponding restore invocation to remove "|| true", capture the command's
exit status immediately (e.g., via $? or using an if/then construct around the
invocation), assert/fail the test if the exit code is non-zero before
proceeding, and only then validate the stdout string (e.g., check backup_output
for "Backup saved"); apply the same pattern to the restore invocation so
failures are not hidden.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@test/e2e/test-state-backup-restore.sh`:
- Around line 216-217: The grep checks that look for literal marker strings
(using grep -q "${marker_content}_${f}") can false-match due to regex
metacharacters; update these to use fixed-string matching (grep -F or grep
--fixed-strings) so the checks are literal, and keep the conditional logic that
increments backup_files_ok and other similar checks intact; apply the same
change for the other occurrences referenced (the checks around lines 233-234,
289-290, and 307) that use marker_content and backup_dir variables.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/e2e/test-state-backup-restore.sh`:
- Around line 307-310: The current success check for the memory restore probe
only looks for the marker in memory_probe; update the conditional to require
both STATE=EXISTS and the marker match: when testing memory_probe, first ensure
grep -q "^STATE=EXISTS" matches and also grep -Fq -- "${marker_content}_daily"
matches before calling pass ("TC-STATE-01..." ); keep the existing STATE=MISSING
branch that calls print_restore_output_for_diag("$restore_output") and ensure
any other non-EXISTS/non-MISSING case is handled consistently (e.g., treated as
failure) so restore success strictly requires STATE=EXISTS plus marker presence,
referencing memory_probe and marker_content in the change.
- Line 203: The current assignment to backup_dir uses lexical sort which can
pick the wrong directory; change it to select the most-recently modified
directory by sorting on mtime instead of name: build the find pipeline that
prints each directory's modification timestamp with its path, sort numerically
descending (newest first), take the first entry and extract just the path, and
assign that to the backup_dir variable so backup_dir always points to the most
recently modified backup directory.