chore: add handshake stage diagnostics

[vincentkoc]

Problem

A connection can stop between authentication submission, hello validation and session persistence without a role-scoped diagnostic identifying the boundary. The cause of the observed missing operator session is not yet known.

Changes

• Record failure entry before transport cleanup, using a fixed role, the existing reason enum and numeric/named local error.
• Record response correlation and hello-validation facts without raw request IDs or payloads. Valid IDs retain the existing state-lock boundary; invalid IDs do not acquire it.
• Record operator handoff presence separately from an actual sibling-role store result.
• Record the own-session store result after releasing the existing state lock. A stale/ignored response reports store_attempted=0, not a successful store.

Production changes are confined to esp_openclaw_node_runtime.c and esp_openclaw_node_protocol.c. No public API, extra NVS reads, retries, timers, acceptance rules or storage-policy changes.

Dependency

Depends on the now-landed #39.
The original diagnostic delta, based on c7c7a0c8cb2178ceba41af03e4b3a8c10f3b6f8a,
is preserved. The PR now targets main with the reviewed UI and firmware-provenance
changes inherited, not duplicated.

Original Validation

• Local firmware packaging tests: 8 passed; the SDK-only lock-reader case was skipped locally.
• Actionlint and git diff --check passed.
• Fresh independent review of the frozen delta found no actionable P0-P2 defects.
• One table-driven case extends the existing target Unity fixture: accepted hello, missing operator handoff, own persistence failure, handoff load failure and rejection. It checks event/persistence outcomes, diagnostic-before-cleanup ordering and synthetic credential canaries without golden log snapshots.
• The new Unity case compiled successfully but has not been executed. Existing CI builds the core test app without running it; no local IDF installation or new host shim was added.
• All five builds passed in exact-head CI run 34182849942: component test app, generic ESP32, BOX3, Tab5 P4 and Waveshare S3.

Firmware Artifact

• Tab5 artifact 10039648607 was downloaded and verified: 2893649 bytes, SHA-256 ce103b02f1d73f088fff3cd52a2d02142d0239b2663c554fa5bc503d8b0f548f, matching the GitHub API artifact digest.
• All 11 checksum entries and 10 manifest file records match. The four image offsets are 0x2000, 0x8000, 0x10000 and 0x810000; sizes, hashes and non-overlap within 16 MiB were verified.
• Compiled test merge fe1938e2be977651ad185b6b91fc47283f85bc3b has parents c7c7a0c8cb2178ceba41af03e4b3a8c10f3b6f8a and PR head 27cbf3dde7174fe37c1c3e06bf66062d815dd117. Its Git tree equals the PR head's tree.
• Actual IDF is 362a1776ec212788fda95f75b733bfdde3a0c394, build revision v5.5.5-648-g362a1776ec2, with esptool 4.12.0. This matches the prior Tab5 build; it is not pristine release v5.5.5, and CI still inherits the moving SDK lane.
• Only the Tab5 bundle was downloaded. This is artifact integrity and compilation proof, not reproducibility or physical diagnostic qualification.

Limits

This is diagnostic instrumentation, not a repair or proof of root cause. No device, Gateway, provider or credential access was performed for this change. Physical flashing and the next bounded connection trace belong to separate validation.

The credential-free claim applies only to the new connect_diag records. Existing logs are not globally redacted; raw serial output must not be treated as safe to publish. Added logging can affect timing, and a successful later connection alone would not prove a fix.

Landing Preparation

Signed head 905aa25ebca4cbfeaf01541b2ef0452ac138205d reconciles the original
diagnostics onto main 407dc9e59d351cdbc084e66be82bec675d7fcf07 while retaining
the original signed diagnostic and node-home repair ancestry. The complete
tree is exactly that main plus the original diagnostic delta; no new
diagnostic behavior was introduced. All five builds and CodeQL passed at this
head in CI run 34505634202.
Maintainer disposition on September 12, 2026: accept the reviewed
instrumentation with compilation-only validation of the target Unity case.
That case has not been executed; this is not a claim that its runtime assertions
passed. The source review, exact-head builds and full-tree/parent checks support
landing this bounded diagnostic change independently of unresolved voice
qualification. Landing uses a merge commit and retains the original signed
commit objects.

[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.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 12, 2026, 12:46 AM ET / 04:46 UTC (Revision 6).

ClawSweeper review

What this changes

Adds role-scoped ESP32 connection-stage logs and a five-scenario Unity case covering diagnostic content, cleanup ordering, and connection outcomes.

Merge readiness

✅ Ready for maintainer review

Ready for ordinary maintainer review with no actionable findings. The diagnostics remain absent from main, and the September 12 maintainer disposition resolves the previous request to execute the Unity case or explicitly accept compilation-only validation.

Priority: P3
Reviewed head: 905aa25ebca4cbfeaf01541b2ef0452ac138205d

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, source-reviewed diagnostic patch with successful head builds and an explicitly accepted runtime-validation limitation.
Proof confidence	🌊 off-meta tidepool	Not applicable: The MEMBER-authored instrumentation is exempt from the external-contributor runtime-proof gate. Its response-processing and failure-cleanup Unity assertions remain unexecuted; the captured maintainer disposition explicitly accepts compilation-only validation. Artifact integrity and successful builds are not runtime proof.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The MEMBER-authored instrumentation is exempt from the external-contributor runtime-proof gate. Its response-processing and failure-cleanup Unity assertions remain unexecuted; the captured maintainer disposition explicitly accepts compilation-only validation. Artifact integrity and successful builds are not runtime proof.
Evidence reviewed	7 items	Introduced scope: The pinned main-to-head delta contains only two production files and one test file: production +88/-8 and tests +222/-0. Existing behavior and necessity: Main already implements role-token handoff and session finalization but lacks the proposed stage records. The patch preserves their acceptance rules, storage calls, and state-lock boundaries. Diagnostic safety: Failure diagnostics precede cleanup and contain fixed role names, reason enums, and local error codes. Protocol diagnostics use presence and correlation facts rather than raw tokens, URLs, request IDs, or Gateway error text.
Findings	None	None.
Security	None	None.

How this fits together

The ESP32 node component receives Gateway connection responses over WebSocket, validates them, and persists reconnect sessions. These diagnostics expose progress through that process before the application receives a connected or failed event.

flowchart TD
  A[Gateway response] --> B[Response correlation]
  B --> C[Hello validation]
  C --> D[Role session persistence]
  D --> E[Application connection event]
  B --> F[Stage diagnostics]
  C --> F
  D --> F
  C --> G[Failure cleanup]
  G --> E
  G --> F

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +88/-8; tests +222/-0	Production growth serves the stated diagnostic boundaries; test growth adds capture helpers and five scenarios.

Technical review

Best possible solution:

Retain the bounded stage diagnostics while keeping root-cause and physical-device qualification claims separate.

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

Not applicable to the instrumentation itself; the underlying missing-operator-session failure remains unexplained, and the new Unity case has compiled but has not run.

Is this the best way to solve the issue?

Yes. Instrumenting the existing boundaries is a narrow way to distinguish failure stages without introducing retries, storage-policy changes, or competing connection logic.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 407dc9e59d35.

Labels

Label justifications:

• P3: This is bounded diagnostic instrumentation without a demonstrated change to connection outcomes.
• 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-authored instrumentation is exempt from the external-contributor runtime-proof gate. Its response-processing and failure-cleanup Unity assertions remain unexecuted; the captured maintainer disposition explicitly accepts compilation-only validation. Artifact integrity and successful builds are not runtime proof.

Evidence

What I checked:

• Introduced scope: The pinned main-to-head delta contains only two production files and one test file: production +88/-8 and tests +222/-0. (905aa25ebca4)
• Existing behavior and necessity: Main already implements role-token handoff and session finalization but lacks the proposed stage records. The patch preserves their acceptance rules, storage calls, and state-lock boundaries. (components/esp-openclaw-node/src/esp_openclaw_node_protocol.c:468, 407dc9e59d35)
• Diagnostic safety: Failure diagnostics precede cleanup and contain fixed role names, reason enums, and local error codes. Protocol diagnostics use presence and correlation facts rather than raw tokens, URLs, request IDs, or Gateway error text. (components/esp-openclaw-node/src/esp_openclaw_node_runtime.c:23, 905aa25ebca4)
• Prior review disposition: The supplied previous review had no findings and requested Unity execution or explicit acceptance without execution. The captured PR body records MEMBER-author maintainer acceptance on September 12, while explicitly retaining the unexecuted-test limitation. Snapshot sourceRevision: 5c9bd5e5013817922e07e76675bba8bc641f625f1707d5b6b84dd332e9327679. (905aa25ebca4)
• Head validation: GitHub reports the referenced CI run completed successfully at the exact reviewed head. The supplied check inventory includes five successful builds and CodeQL; compilation does not establish execution of runtime assertions. (905aa25ebca4)
• Related work and release check: The supplied GitHub context identifies feat: add a static Tab5 node home #39 as the merged display prerequisite and ci: retain matching Tab5 build symbols #41 as separate, open symbol-retention work. Neither replaces the diagnostics. The releases endpoint returned no published releases.

Likely related people:

• vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

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 (5 earlier review cycles)

• reviewed 2026-09-08T03:17:07.702Z sha 27cbf3d :: blocked before merge. :: none
• reviewed 2026-09-08T03:31:08.641Z sha 27cbf3d :: blocked before merge. :: none
• reviewed 2026-09-10T16:49:30.875Z sha 27cbf3d :: blocked before merge. :: none
• reviewed 2026-09-10T17:04:53.297Z sha 905aa25 :: needs maintainer review before merge. :: none
• reviewed 2026-09-10T17:17:28.721Z sha 905aa25 :: blocked before merge. :: none