fix: recognize P4 SPM stacks in flash guard

[vincentkoc]

Problem

On the connected M5Stack Tab5 ESP32-P4 v1.3, flash access asserted at cache_utils.c:127 with a task stack pointer inside internal SPM. The selected ESP-IDF base accepts SPM for internal allocations but omits it from its flash-operation stack-sanity predicate. This matches espressif/esp-idf#19020.

Stacked on #41 to retain the exact normal ELF/map alongside the firmware. This does not merge any dependency PR.

Changes

• Add only the SOC_MEM_SPM_SUPPORTED-guarded esp_ptr_in_spm(sp) case to the stack-sanity predicate. Keep the assertion, DRAM/RTC checks, allocation policy, NVS behavior, and chip-revision selections unchanged.
• Apply the tracked patch only in Tab5's isolated CI SDK container before configuration. Manual application requires an explicitly selected, dedicated SDK checkout.
• Require SDK base 362a1776ec212788fda95f75b733bfdde3a0c394, the exact tracked patch, the original whole-file hash, and the exact resulting source hash. Reject unrelated tracked/untracked changes, another base, staged changes, and tampering. Verify an already-applied exact patch without reapplying it.
• Preserve the packager's dirty-SDK refusal for other examples. The Tab5 exception independently validates the actual SDK source and records dirty state, base commit, patch hash, and patched-source hash. Firmware and symbol artifacts share the same manifest.
• Document the isolated manual procedure and removal condition. This is ESP-IDF plus a tracked compatibility patch, not pristine ESP-IDF 5.5.5 or a claim that PSRAM stacks are safe for flash access.

Original Checks

• Ten SDK-helper tests passed using real Git repositories and patch application: acceptance, idempotence, verification-only refusal, wrong base, unrelated tracked/untracked changes, patch/source tampering, staged changes, and invalid SDK-root selection.
• Twelve firmware-packaging tests passed; one existing dependency-lock reader test was skipped locally because its YAML dependency is supplied by IDF CI.
• Hosted CI reran the ten helper and twelve packaging tests successfully, then passed the real dependency-lock reader test inside the IDF container.
• The tracked patch applied cleanly to the independently captured original SDK file and produced the pinned patched-source hash.
• actionlint and git diff --check passed.
• Independent scoped review found no actionable P0-P2 findings.
• All five jobs passed in run 34190325714, attempt 1 at PR head c8e63f6b5510156ba1b12dc4e916031823a39056, including Tab5 firmware and symbol uploads.
• The completed Tab5 firmware and symbols ZIPs match GitHub artifact digests (10042137041 and 10042137904). All 11 firmware and 3 symbol checksum entries passed; the four images retain offsets 0x2000, 0x8000, 0x10000, and 0x810000.
• Both manifests are byte-identical and record compiled merge 2949820b310a0e14d3317fd1ffb37f9f6fadd4c0, with the expected PR/base parents and the same tree as the PR head.
• The retained ELF's full SHA256 matches the app's embedded ELF SHA256: 3ca692fc85a1259414e8f9cc9ca571fdfe47feaed08b6f96e11559a54c5ad9a3. Embedded app version 2949820 and SDK revision match the manifest.

Qualification Limits

Matched firmware/symbol download verification passed. The manifest records SDK base 362a1776ec212788fda95f75b733bfdde3a0c394, revision v5.5.5-648-g362a1776ec2-dirty, and the exact approved patch/source hashes. The SDK base matches the preceding build, but its source now includes the tracked compatibility patch. The image selector remains moving and the helper fails closed if its base changes.

The affected connected board is P4 v1.3. Current profile/image evidence (CONFIG_ESP32P4_SELECTS_REV_LESS_V3=y, image minimum revision 0, maximum 199) does not establish same-binary revision 3 or newer compatibility. These checks do not qualify other P4 revisions or the new ancestry-prepared image physically. The existing defaults comment suggesting "newer silicon" needs a separately qualified correction; this change does not alter revision selections.

No device access, flashing, USB-JTAG, Gateway, or provider calls were performed for this implementation. A successful build would not by itself prove that the observed assertion is resolved on hardware.

Landing Preparation

Signed head ba3437946360327d5e6c7dc386ad9fe50a7f9ce2 retains the original
SPM compatibility commit and incorporates the reviewed predecessor
1d1c83af134b16824cbbeec940f834419c1e51ba. Its complete tree is exactly that
predecessor plus the original compatibility delta, preserving the landed
release guard and node-home repair. No new SDK patch or runtime policy was
introduced. All five builds passed in
exact-head CI run 34507229544;
the artifact evidence above belongs to the original build.

Maintainer disposition on September 12, 2026: accept the narrow compatibility
fix and its intentional fail-closed refusal when the moving SDK selector stops
matching the approved base. SDK pinning is separate work; this PR does not
weaken the identity guard to keep builds running.

Separate campaign evidence on the matched 2949820 image recorded four flash
readbacks, successful node-session persistence and 60 seconds without the SPM
assertion or reset. Later operator/NVS failures remained separate unresolved
outcomes; this is not complete storage, other-silicon, or voice qualification.
No new hardware test was performed for this ancestry-prepared head. Landing
uses a merge commit and retains the original signed 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: blocked before merge. Reviewed September 12, 2026, 1:25 AM ET / 05:25 UTC (Revision 5).

ClawSweeper review

What this changes

Adds a verified ESP-IDF patch that accepts internal SPM task stacks during Tab5 flash operations, with isolated CI application, firmware provenance, documentation, and refusal tests.

Merge readiness

⛔ Blocked before merge - 2 items remain

Still needed and suitable to land: main lacks the compatibility fix, no actionable patch defect was found, and the recorded maintainer disposition resolves both prior review follow-ups.

Priority: P2
Reviewed head: ba3437946360327d5e6c7dc386ad9fe50a7f9ce2

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, defensively verified repair with no actionable findings and explicit acceptance of its remaining qualification limits.
Proof confidence	🌊 off-meta tidepool	Not applicable: The ordinary contributor-proof gate does not apply to this MEMBER-authored PR. Its recorded disposition accepts bounded P4 v1.3 flash/persistence campaign results for the unchanged SDK patch; the prepared head has build evidence, not a new hardware run.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The ordinary contributor-proof gate does not apply to this MEMBER-authored PR. Its recorded disposition accepts bounded P4 v1.3 flash/persistence campaign results for the unchanged SDK patch; the prepared head has build evidence, not a new hardware run.
Evidence reviewed	9 items	Review scope and policy: The remote identifies the target repository. No root or nested AGENTS.md or maintainer-note files were found; the checkout remained clean. Verified introduced scope: The pinned merge-base delta contains seven files. Comparing the original reviewed head with this head shows no changes to the SDK patch, helper, or packager. The host-verified test merge has the exact main and PR-head parents. SDK dependency and integrity boundary: The tracked patch directly modifies ESP-IDF’s flash safety predicate, establishing the dependency contract. The helper checks the SDK base, patch hash, whole-source hashes, regular source path, and exact worktree status before accepting the compatibility delta; packaging grants this exception only to Tab5.
Findings	None	None.
Security	None	None.

How this fits together

Tab5 firmware uses ESP-IDF to access flash for operations such as session persistence. This change adjusts the SDK’s stack safety check and records the exact patched SDK identity in downloadable firmware artifacts.

flowchart TD
  A[Tab5 CI build] --> B[Verify SDK and patch identity]
  B --> C[Apply isolated compatibility patch]
  C --> D[Build firmware]
  D --> E[Verify SDK delta and package artifacts]
  D --> F[Device flash operation]
  F --> G[Accept internal SPM stack]

Loading

Before merge

• Resolve merge risk (P1) - When the moving release-v5.5 image changes SDK commit, Tab5 CI will stop before compilation and produce no firmware or symbols; the September 12 maintainer disposition explicitly accepts this interruption.
• Resolve merge risk (P1) - The reported hardware observations cover the earlier matched P4 v1.3 image, not the ancestry-prepared image, other silicon revisions, or complete storage qualification.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test growth	Production/tooling +115 net; tests +202 net; documentation +40	The tooling growth supports exact SDK verification and auditable packaging around a three-line SDK predicate change.

Merge-risk options

Maintainer options:

1. Honor the accepted SDK refusal policy (recommended)
   Land with the exact-base guard intact and handle SDK image pinning or future qualification separately, as the maintainer disposition specifies.

Technical review

Best possible solution:

Retain the exact SDK identity guard under the accepted interruption policy, then retire the compatibility patch when an upstream-fixed SDK is selected and qualified.

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

Yes, the source establishes the failure path: a valid internal SPM task stack reaches a flash predicate that accepts only DRAM/optional RTC memory. This review did not execute a current-main hardware reproduction.

Is this the best way to solve the issue?

Yes. Extending only the flash predicate preserves existing allocation and assertion behavior; the exact-source checks keep the local SDK exception narrow and auditable.

AGENTS.md: not found in the target repository.

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

Labels

Label justifications:

• P2: This is a focused repair for Tab5 flash assertions with bounded hardware and SDK scope.
• merge-risk: 🚨 automation: The introduced exact-base guard intentionally stops Tab5 builds when the existing moving SDK selector advances.
• 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 ordinary contributor-proof gate does not apply to this MEMBER-authored PR. Its recorded disposition accepts bounded P4 v1.3 flash/persistence campaign results for the unchanged SDK patch; the prepared head has build evidence, not a new hardware run.

Evidence

What I checked:

• Review scope and policy: The remote identifies the target repository. No root or nested AGENTS.md or maintainer-note files were found; the checkout remained clean. (ba3437946360)
• Verified introduced scope: The pinned merge-base delta contains seven files. Comparing the original reviewed head with this head shows no changes to the SDK patch, helper, or packager. The host-verified test merge has the exact main and PR-head parents. (patches/esp-idf/tab5-spm-stack-sanity.patch:9, ba3437946360)
• SDK dependency and integrity boundary: The tracked patch directly modifies ESP-IDF’s flash safety predicate, establishing the dependency contract. The helper checks the SDK base, patch hash, whole-source hashes, regular source path, and exact worktree status before accepting the compatibility delta; packaging grants this exception only to Tab5. (scripts/idf_tab5_compat.py:32, ba3437946360)
• Authoritative internal-memory and stack contract: At the pinned SDK revision, esp_ptr_internal includes SPM and the general stack predicate accepts aligned SPM stacks, while cache_utils.c’s flash predicate omits SPM. The heap layout also exposes SPM with MALLOC_CAP_INTERNAL. This supports the narrow predicate repair without changing allocation policy. (components/esp_hw_support/include/esp_memory_utils.h:417, 362a1776ec21)
• Upstream corroboration: The open upstream report describes the same P4 v1.3 SPM-stack assertion and explains why increasing stack size merely avoids the affected allocation: ESP32-P4: main_task stack can be allocated from SPM heap region, invisible to esp_ptr_in_dram(), causing spurious cache-disable assert (IDFGH-18199) espressif/esp-idf#19020.
• Main and release necessity check: Fetched main still builds Tab5 without applying this patch and rejects dirty SDKs unconditionally. The merged symbol-retention predecessor does not fix the assertion. The GitHub releases endpoint returned no releases, so no shipped fix is established. (.github/workflows/ci.yml:78, d7c4a40fc71c)

Likely related people:

• vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Peter Steinberger: 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 (4 earlier review cycles)

• reviewed 2026-09-08T05:24:19.528Z sha c8e63f6 :: blocked before merge. :: none
• reviewed 2026-09-08T05:56:35.331Z sha c8e63f6 :: blocked before merge. :: none
• reviewed 2026-09-10T17:21:22.034Z sha ba34379 :: blocked before merge. :: none
• reviewed 2026-09-12T05:19:24.105Z sha ba34379 :: blocked before merge. :: none