diag(tab5): capture first NVS and session-load failures

[vincentkoc]

Problem

Saved-session absence can follow several different NVS outcomes. Existing
normalization and clearing paths do not expose the first underlying failure.
The cause remains unknown; this is diagnostic instrumentation, not a repair.

Stacked on #43, base
fix/tab5-psram-allocation-policy at
d4aae3f2f07f7d2f8a59b9df1b73c63849df6edd.

Changes

• Add a temporary, explicitly selected Tab5 SDK diagnostic patch. The five
  NVSPartition methods capture one failing result per boot, including existing
  alignment rejection, using a lock-free internal-RAM claim and heap-free ROM
  output after the operation returns.
• Record fixed numeric session-load role/stage/error/classification before
  normalization or clearing; record the clear result separately. Observe cached
  and fresh session presence during saved reconnect, and initial NVS init errors
  before the existing recovery path.
• Preserve original returns, no-I/O alignment behavior, session/cache ownership,
  clearing/recovery policy, and connection behavior. No retries, tasks, commands,
  persistent fields, or recovery fixes are added.
• Keep SDK base 362a1776ec212788fda95f75b733bfdde3a0c394, the existing SPM patch
  bytes, allocation cutoff 1024, reserve 32768, and other configuration unchanged.
  This is ESP-IDF plus two tracked patches, not pristine ESP-IDF.
• Require explicit diagnostic mode for the exact two SDK source deltas.
  Firmware provenance records both patches and whole-source hashes; the separate
  symbol artifact copies the same manifest.

New diagnostic records contain only fixed labels and numeric fields, without
keys, lengths, offsets, URLs, IDs, tokens, or payloads. They are unprefixed ROM
lines; consumers must capture them before normal I/W/E filtering. Output can
interleave, and missing records do not establish healthy NVS. SDK instrumentation
is Tab5-only; session/init observations are in the existing shared components.

Validation

• 12 SDK helper tests passed: exact-base/delta verification, refusal, and idempotence.

• 14 packaging tests passed; the local YAML-reader case is skipped and remains
  exercised inside the existing IDF CI environment.

• Host diagnostic tests passed against the actual patched SDK methods and actual
  session loader: original returns, alignment without I/O, concurrent single
  capture, missing/empty/unsupported/malformed sessions, clear failure, and
  synthetic canary non-disclosure.

• Added one cached-session retention regression to the existing target Unity
  suite. Target tests are built by CI, not executed here.

• Actionlint and git diff --check passed. Fresh independent scoped review
  through P2 reported no actionable findings.

• CI run 34247823770, attempt 1
  passed all five build jobs at head
  c6d56b5dfa8718f5b7822919d0459824bf857336, including the component test app
  and Waveshare host lifecycle/UI checks.

• The follow-up commit changes only three test files: mutable session fixture
  arrays and a format-checked host ROM-output shim. These correct the two initial
  CI failures without changing production code, SDK patches, or configuration.
  Focused review found no actionable issues. Full lifecycle execution was
  unavailable locally because managed dependencies were absent; CI ran it.

The earlier head 71febcbea6d88fb51e63720489e8267beff1e326 produced test-merge
image 3d22ae8616a34b670a5c947e25e9838caf8292dd in
run 34233725476.
Its Tab5 archive (artifact 10059256231, SHA256
17a2e5818161a7e24d41b98487d5d7399b3d5f191c9993e3251882d0273d2775)
was checked against the API digest, all 11 file checksums, source-parent
linkage, four image offsets, unchanged configuration, and both exact SDK
patch records. Its flash ranges exclude NVS. The four images were subsequently
flashed and readback-verified on M5Stack Tab5.

That is historical-image evidence, not a claim that the latest tests-only head
was flashed. A subsequent SDIO receive-buffer assertion remains unresolved;
this diagnostic PR does not claim to repair it or qualify a complete Talk flow.
No JTAG or release was performed during that verification.

Maintainer Disposition

On September 12, 2026, accept this bounded diagnostic change using its existing
source review, host regression tests, exact-head native builds, and the
historical-image evidence above. Target Unity fixtures were compiled, not
executed. First-failure instrumentation does not establish healthy NVS when no
record is observed, and this PR does not repair session loss or qualify voice.
The exact SDK refusal policy remains intact. Land with a merge commit and
retain the signed source objects; no release or new hardware test is implied.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 12, 2026, 1:54 AM ET / 05:54 UTC (Revision 4).

ClawSweeper review

What this changes

Adds numeric flash-storage and saved-session diagnostics, an opt-in Tab5 SDK patch, firmware provenance records, and regression coverage.

Merge readiness

✅ Ready for maintainer review

Keep open as a useful landing candidate: current main lacks these diagnostics, the prior host-harness defect is fixed, and the maintainer explicitly accepted the bounded validation. No remaining actionable patch defect was found.

Priority: P2
Reviewed head: c6d56b5dfa8718f5b7822919d0459824bf857336

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, maintainable diagnostic patch with meaningful regression coverage and no remaining actionable finding.
Proof confidence	🌊 off-meta tidepool	Not applicable: The member-author exemption applies. NVSPartition and session-loader tests use I/O stubs; the body separately records historical Tab5 flash/readback verification and current-head native builds, without claiming observed hardware first-failure capture. The maintainer explicitly accepted those limits.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The member-author exemption applies. NVSPartition and session-loader tests use I/O stubs; the body separately records historical Tab5 flash/readback verification and current-head native builds, without claiming observed hardware first-failure capture. The maintainer explicitly accepted those limits.
Evidence reviewed	10 items	Current main still lacks the diagnostic behavior: Inspected main's complete session loader and searched its source, scripts, patches, and examples. Existing clearing and normalization remain, but the four new diagnostic record types are absent. The merged allocation-policy prerequisite does not implement this instrumentation. Introduced scope and merge ownership: Reviewed all 17 introduced files. Raw test-merge parents are the pinned main followed by the exact PR head; comparison against its main parent contains only those 17 files, preserving the unrelated main-only work. Storage behavior and credential handling remain intact: The introduced session and reconnect hunks add numeric observations without changing stored keys, version, clearing policy, cached-session fallback, or connection authorization. The SDK patch returns each original result and retains no-I/O alignment rejection.
Findings	None	None.
Security	None	None.

How this fits together

ESP device firmware loads saved Gateway sessions from nonvolatile storage during startup and reconnect. These diagnostics expose storage failures before existing normalization and recovery, while packaging records the exact patched SDK.

flowchart TD
  A[Startup or saved reconnect] --> B[Session loader]
  B --> C[NVS flash operations]
  C --> D[First failure capture]
  B --> E[Session outcome diagnostics]
  D --> F[Numeric console records]
  E --> F
  B --> G[Existing session recovery and connection]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Net line growth	Runtime +81; build scripts +44; SDK patch +74; tests/fixtures +535; docs +52; CI +4	Production growth supports the stated diagnostic and provenance scope, with most additions devoted to regression fixtures.

Technical review

Best possible solution:

Keep the instrumentation temporary and observational, preserve exact SDK provenance, and retire the diagnostic mode once the underlying failure is identified and qualified.

Do we have a high-confidence way to reproduce the issue?

Not applicable to this diagnostic-only change: the underlying field session-loss cause remains unestablished, and this review did not execute firmware.

Is this the best way to solve the issue?

Yes. Numeric observations before normalization preserve existing behavior, and the explicit SDK mode extends the established provenance mechanism without a competing recovery path.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 9b46a78f838d.

Labels

Label justifications:

• P2: Bounded diagnostic instrumentation helps investigate observed Tab5 storage failures without claiming an urgent runtime repair.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The member-author exemption applies. NVSPartition and session-loader tests use I/O stubs; the body separately records historical Tab5 flash/readback verification and current-head native builds, without claiming observed hardware first-failure capture. The maintainer explicitly accepted those limits.

Evidence

What I checked:

• Current main still lacks the diagnostic behavior: Inspected main's complete session loader and searched its source, scripts, patches, and examples. Existing clearing and normalization remain, but the four new diagnostic record types are absent. The merged allocation-policy prerequisite does not implement this instrumentation. (components/esp-openclaw-node/src/esp_openclaw_node_persisted_session.c:251, 9b46a78f838d)
• Introduced scope and merge ownership: Reviewed all 17 introduced files. Raw test-merge parents are the pinned main followed by the exact PR head; comparison against its main parent contains only those 17 files, preserving the unrelated main-only work. (29dd58f0a1c0)
• Storage behavior and credential handling remain intact: The introduced session and reconnect hunks add numeric observations without changing stored keys, version, clearing policy, cached-session fallback, or connection authorization. The SDK patch returns each original result and retains no-I/O alignment rejection. (components/esp-openclaw-node/src/esp_openclaw_node_persisted_session.c:58, c6d56b5dfa87)
• Explicit SDK dependency and bounded packaging exception: The patch directly modifies ESP-IDF's NVSPartition implementation, establishing the dependency boundary. Verification requires the pinned SDK commit, exact approved dirty paths, and original/patched whole-file hashes; packaging preserves compatibility_patch and adds diagnostic_patch only in explicit diagnostic mode. (scripts/idf_tab5_compat.py:71, c6d56b5dfa87)
• Pinned upstream implementation inspected: Read the exact upstream NVSPartition source through GitHub. Its five partition calls and two alignment checks match the fixture and patch context; the proposed wrappers preserve those calls and returns. (components/nvs_flash/src/nvs_partition.cpp:26, 362a1776ec21)
• Prior finding resolved: GitHub's exact-head commit diff confirms the follow-up changes only three test files, including the previously missing ROM-output declaration and implementation for the lifecycle host harness. (components/esp-openclaw-room-node/tests/host/esp_rom_sys.h:1, c6d56b5dfa87)

Likely related people:

• Vincent Koc: Raw commit c8e63f6 adds scripts/idf_tab5_compat.py:37 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: c8e63f6b5510; files: scripts/idf_tab5_compat.py)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)

• reviewed 2026-09-08T13:46:25.427Z sha 71febcb :: blocked before merge. :: [P2] Provide the ROM logging boundary to the lifecycle host harness
• reviewed 2026-09-08T16:10:27.544Z sha c6d56b5 :: needs changes before merge. :: none
• reviewed 2026-09-12T05:47:23.047Z sha c6d56b5 :: needs maintainer review before merge. :: none

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.