fix(rollback): enumerate persona before inventory-match

[cbeaulieu-gt]

Summary

Closes #213

Step 6.3.5b in .github/workflows/runtime-rollback.yml was passing the image URL directly as $1 to runtime/scripts/inventory-match.sh. That script's contract (line 36) requires a local JSON file — the output of enumerate-persona.sh. Passing a URL caused an immediate ERROR enumeration_json_not_found at line 39, failing every rollback dispatch.

• Add enumerate-persona.sh call per overlay inside the loop, writing to a mktemp file
• Pass the temp file (not the image URL) as $1 to inventory-match.sh
• Forward SMOKE_UID from steps.uid.outputs.uid via the step env: block (required by enumerate-persona.sh)
• Mirrors the established pattern in runtime/scripts/overlay-smoke.sh lines 66–74

Verification

• YAML parse: python -c "import yaml; yaml.safe_load(open(...))" — clean
• actionlint: actionlint .github/workflows/runtime-rollback.yml — no output (clean pass)
• Manual diff review: confirmed (a) enumerate-persona.sh called per overlay, (b) output captured to mktemp file, (c) that path passed as $1 to inventory-match.sh, (d) for ov in review fix explain loop preserved, (e) all three overlays exercised
• shellcheck not on PATH; no SC2016 cases introduced (no ${{ }} in single-quoted strings in the modified block)

Post-merge follow-up

Once this lands, re-dispatch Task 6.9 rehearsal Step 6.9.4′ with target_pubsha=b080630804cc5783be7060121a05db1b1523ad0f (Set B — known-good, in promote history at commit 7a061b5) to confirm the live fix. Step 6.3.5c (Validate target digests via smoke-test) was left untouched — it runs separately and provides complementary coverage via smoke-test.sh.

🤖 Generated by Claude Code on behalf of @cbeaulieu-gt

[github-actions]

Claude finished @cbeaulieu-gt's task in 2m 14s —— View job

---

PR Review Complete ✅

Summary

Verdict: LGTM — This is a high-quality fix that correctly addresses the root cause identified in #213. The implementation follows established patterns, includes comprehensive error handling, and is thoroughly documented.

---

Review Findings

✅ Code Quality — Excellent

Strengths:

• Correct fix: The bug was passing an image URL to inventory-match.sh which expects a local JSON file (per inventory-match.sh:36). The fix properly calls enumerate-persona.sh first to generate the required JSON.
• Pattern adherence: Mirrors the established pattern in overlay-smoke.sh:66-74 exactly — same error-code capture approach, same temp file handling, same env var forwarding.
• Clear variable names: ENUM_OUT, enum_rc, match_rc, IMAGE_URL are all self-documenting.
• Proper error handling:
  • Exit codes captured via || rc=$? pattern (not the broken if ! cmd; then rc=$? anti-pattern)
  • Errors include full context (overlay name, image URL, exit code)
  • Temp files cleaned up immediately after use with rm -f
• Excellent inline documentation: Lines 181-184 explain what the script expects, what the bug was, what the fix is, and reference runtime-rollback.yml Step 6.3.5b passes image URL to inventory-match.sh — needs enumerate-persona.sh extraction first #213.

Minor observations:

• No trap cleanup EXIT for temp file cleanup, but this is consistent with overlay-smoke.sh and the workflow job cleanup will handle orphaned temp files anyway.
• The chmod +x line correctly adds both scripts (line 185).

✅ Security — No Issues

• ✅ No secrets in code
• ✅ mktemp used for secure temp file creation
• ✅ Input validation happens in earlier steps (digest format, pubsha shape)
• ✅ set -euo pipefail enforces fail-fast shell scripting
• ✅ No injection risks — $ov is loop-controlled to review|fix|explain, $DIGEST is validated in step "Resolve target digests"
• ✅ Env var forwarding uses explicit quoting: IMAGE_REF="$IMAGE_URL" SMOKE_UID="$SMOKE_UID"

✅ Performance — Efficient

• ✅ Enumerate called once per overlay (no redundant Docker runs)
• ✅ Image already pulled in "Resolve target digests" step — no duplicate pulls
• ✅ Temp files created/cleaned per iteration — minimal disk footprint
• ✅ No blocking calls, N+1 queries, or inefficient algorithms

⚠️ Test Coverage — Manual Verification Only

What was tested:

• ✅ YAML syntax validation (python -c "import yaml; yaml.safe_load(...)")
• ✅ actionlint clean pass
• ✅ Manual diff review confirming all aspects of the fix

Gap:

• ⚠️ No automated test coverage for this workflow — but this is consistent with the entire .github/workflows/ tree (no test suite exists for workflows)
• ✅ Post-merge verification plan documented in PR body (Step 6.9.4′ rehearsal with Set B)

Assessment: Test coverage is appropriate for this change. The manual verification is thorough, and the post-merge follow-up will confirm the fix works end-to-end.

✅ Documentation — Comprehensive

• ✅ Inline comment block (lines 181-184): Explains contract, bug, fix, and issue reference
• ✅ PR body: Thorough summary with verification steps and post-merge plan
• ✅ Error messages: Contextual (include overlay name, image URL, exit code)
• ✅ Commit message: Will document the fix in git history

---

Verified Correctness

Contract Compliance

I verified the fix satisfies the script contracts:

1. enumerate-persona.sh requirements (from runtime/scripts/enumerate-persona.sh:47-50):

   • ✅ Receives output file path as $1: "$ENUM_OUT" (line 193)
   • ✅ IMAGE_REF env var set: IMAGE_REF="$IMAGE_URL" (line 192)
   • ✅ SMOKE_UID env var set: SMOKE_UID="$SMOKE_UID" (line 192, sourced from step env line 173)

2. inventory-match.sh requirements (from runtime/scripts/inventory-match.sh:36-37):

   • ✅ Receives enumeration JSON file as $1: "$ENUM_OUT" (line 201)
   • ✅ Receives expected.yaml file as $2: "runtime/overlays/${ov}/expected.yaml" (line 202)

3. Pattern match with overlay-smoke.sh:66-74:

   • ✅ Enumerate first: bash .../enumerate-persona.sh "$ENUM_OUT" || enum_rc=$?
   • ✅ Check exit code, clean up on failure
   • ✅ Match second: bash .../inventory-match.sh "$ENUM_OUT" "$EXPECTED_FILE" || match_rc=$?
   • ✅ Check exit code, clean up, exit on failure

Error Path Coverage

The fix handles all error cases:

• Enumerate failure → error message + cleanup + exit (lines 194-198)
• Match failure → error message + cleanup + exit (lines 204-207)
• Both temp file cleanup paths include rm -f (lines 196, 203)

---

Recommendations

No blocking issues found

This PR is ready to merge. The fix is correct, well-documented, and follows established patterns.

Post-merge action items (already documented in PR body)

Per the PR's "Post-merge follow-up" section, re-dispatch Task 6.9 rehearsal Step 6.9.4′ with target_pubsha=b080630804cc5783be7060121a05db1b1523ad0f to confirm the live fix.

---

Review Checklist

• Code quality: Clear, follows patterns, proper naming ✅
• Security: No injection risks, secrets, or unsafe operations ✅
• Performance: Efficient, no redundant operations ✅
• Test coverage: Manual verification thorough, post-merge plan documented ⚠️ (appropriate)
• Documentation: Inline comments, PR body, error messages all excellent ✅

---

Final recommendation: Approve and merge. This fix correctly resolves #213 with high code quality and comprehensive documentation.