Skip to content

amyboard-pr-preview: keep amy pinned to the PR through the root container build#1066

Merged
bwhitman merged 1 commit into
mainfrom
fix/amyboard-pr-amy-pin
Jul 1, 2026
Merged

amyboard-pr-preview: keep amy pinned to the PR through the root container build#1066
bwhitman merged 1 commit into
mainfrom
fix/amyboard-pr-amy-pin

Conversation

@bwhitman

@bwhitman bwhitman commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Problem

AMY-PR previews (repository_dispatch: amy-pr) started failing in the Build amyboardweb stage/ step (e.g. amy#784):

/bin/sh: 1: cannot create amy/constants.py: Permission denied
make: *** [Makefile:63: src/patches.h] Error 2

Root cause (verified on the runner)

The esp-idf firmware step runs in a Docker container as root with the workspace bind-mounted, and inside it a git submodule update runs (it does git config --global --add safe.directory "*" first). That reverts the amy submodule from the PR's pinned commit back to the superproject's recorded gitlink (main's pin) and leaves the reverted files owned by root:root.

I confirmed this with a diagnostic run: right after the container, amy/amy/constants.py was root root and byte-identical to main's pin (2773 bytes), not the PR's version (2794 bytes with NOTE_SOURCE_MAPPED).

Two consequences:

  1. Correctness (silent): the preview web editor — and very likely the firmware/HW-CI — were built from main's amy, not the PR's. The pin was being undone.
  2. The visible failure: the reverted files are root-owned, so the non-root web build hit Permission denied when make web regenerated amy/amy/constants.py. This only surfaces when the PR changes a header (src/amy.h) without regenerating the committed src/patches.h, so make considers it stale (amy#784 does exactly this); other AMY PRs stayed green while still building main's amy.

Fix

  • Record the PR SHA as the superproject gitlink (git add amy) in the pin step, so the container's git submodule update keeps the PR commit instead of reverting.
  • Reclaim workspace ownership after the root container step so the web build can write (defense-in-depth; the container still root-owns other paths like build/).

Verification

Dispatched this workflow for amy#784 (a07a6ae) on a branch carrying this fix plus a temporary probe + stop-before-deploy guard:

  • After the container: amy HEAD = a07a6ae (PR SHA), constants.py = 2794 bytes, runner runner, HAS PR MARKER — the pin survived and the revert no longer root-owns the tree.
  • Build amyboardweb stage/ passed.
  • The run was intentionally halted before deploy, so no Vercel deploy / HW-CI / PR comment fired (HW-CI gates on conclusion == success).

🤖 Generated with Claude Code

…iner build

The esp-idf firmware step runs in a root Docker container that does a
`git submodule update`, which reverted the `amy` submodule from the PR's
pinned commit back to main's recorded gitlink — so AMY-PR previews (and the
HW CI that flashes them) were silently built from main's amy, not the PR's.
The revert also left amy/ files owned by root, so the later non-root web
build hit `Permission denied` regenerating amy/amy/constants.py whenever the
PR changed a header (e.g. amy#784).

Fix: record the PR SHA as the superproject gitlink (`git add amy`) so the
container's submodule update keeps the PR commit, and reclaim workspace
ownership after the container step so the web build can write.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

🔌 AMYboard PR preview

Editor + flasher: https://amyboard-pr-1066.vercel.app/editor/

This preview bundles this PR's firmware — its flasher only flashes this build (not the release). Rebuilt on every push; removed when the PR closes.

The hardware CI has been kicked off and should return within a few minutes, stand by!

@bwhitman bwhitman merged commit a3a43cb into main Jul 1, 2026
2 checks passed
@bwhitman bwhitman deleted the fix/amyboard-pr-amy-pin branch July 1, 2026 15:00
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

🎛️ HW CI (physical bench)

AMYboard (USB-MIDI + AMY zP → audio): ✅ PASS — flashed this PR’s firmware; all checks matched the references.

Tulip (TULIP4_R11; serial-REPL audio + WiFi screenshot): ✅ PASS — flashed this PR’s firmware; all checks matched the references.

⬇️ Artifacts: recordings · screenshot · serial logs · run logs

Self-hosted bench. Audio spectral-compared to ref/hwci_basic.wav + ref/tulip_basic.wav; Tulip screenshot pixel-compared to ref/tulip_screenshot.png. Both analog outs share one capture card, so the tests run sequentially.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant