Spark hardening: cart test harness, skip-to-content link, cart aria-live announcements

[next-devin]

Three bounded slices sharing one PR, built via the Fable → Codex build loop (Codex CLI built each phase; independent Codex reviews gated each slice).

1. JS test harness for the cart client stack

Plain-Node tests (existing CI pattern, no framework/deps) covering the previously untested modules: SparkCartClient (mocked GraphQL, quantity/subscription edge cases, CSRF/429 retry, expired-cart recovery, timeout/abort contract, event dispatch), SparkCartRewards (gift state transitions, upsell visibility, duplicate gift lookup), spark-progress-bar (shipping/gift threshold events via the public data-value path), spark-cart-drawer (open/close/render + template contract assertions). 33 tests across five files.

2. Skip-to-content accessibility link

Visually hidden "Skip to main content" link as the first focusable element in base.html, revealed on focus, targeting #main-content (<main> in base + homepage override + account-only branch). Localized in all 11 locales. Focused style uses fixed neutral colors (white on slate-800, 14.63:1 contrast) because mid-luminance brand primaries defeat both white text and the data-light-primary override. Compiled main.css rebuilt and committed; source-order/target/compiled-drift tests included.

3. Cart-drawer aria-live announcements

Polite role="status" live region in the drawer shadow DOM announcing on spark:cart:updated — remove → "Item removed from cart.", update → "Cart quantity updated." — wired through SparkI18n and all 11 locales. Repeated identical announcements re-fire via a 0ms-deferred set (cancelled on newer announcements/disconnect). Covered by 5 announcement tests including a repeat-announcement case with deferrable fake timers.

Review trail

• Phase 1 passed Codex review; an --xhigh adversarial challenge found uncovered quantity/retry/gift edges, folded back in as characterization tests (19 → 28 tests).
• Phase 2 failed two review rounds (homepage <main> override lacked the skip target; locale-shape contract broke the Python suite; account-only branch had no target; contrast edge) — all repaired and re-reviewed clear.
• Phase 3 review flagged announcement coalescing; fixed and re-reviewed clear.

The challenge also surfaced four latent gift-flow race conditions in existing source (duplicate gift add on threshold crossing, no reconciliation between threshold edges, unguarded in-flight gift mutations, stale gift flags after empty render). Source fixes are out of scope for this test-and-a11y PR and tracked as follow-up.

Verification

• Full CI-parity run green locally: YAML validation, python3 -m unittest discover -s tests, every tests/js/*.test.js under Node 22, sass-compat --check.
• No live store touched; no ntk push; compiled CSS matches source.

🤖 Generated with Claude Code

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Re-review of commit 47dc28d against incremental diff 8c1456d..47dc28d (2 files, +59/-13).

All four findings from the previous review at commit 8c1456d6 are resolved on changed lines:

File	Line	Previous finding	Resolution
assets/js/components/spark-cart-drawer.js	514	WARNING setTimeout(..., 0) coalescing	Replaced with double requestAnimationFrame; setTimeout(0) retained only as fallback when rAF is unavailable (SSR / test env). The vm-based test harness has no requestAnimationFrame in context, so flushTimers() continues to exercise the fallback.
assets/js/components/spark-cart-drawer.js	422	disconnectedCallback only cleared setTimeout	Now routes through _cancelAnnounce(), which cancels both _announceTimer and _announceRaf (with cancelAnimationFrame guard).
tests/js/skip-to-content.test.js	89-111	WARNING <main>-only scanner missed other tags	New testEveryMainContentTargetIsFocusable scans templates/, layouts/, and partials/ for any element bearing id="main-content" and asserts tabindex="-1". The existing <main>-only scan is preserved for the reverse invariant.
tests/js/skip-to-content.test.js	144-147	SUGGESTION contrast colors not locked	testSkipLinkStylesAndCompiledDriftGuard now asserts color:#fff!important and background-color:#1e293b on the compiled focus rule, locking the 14.63:1 contrast fix.

The SparkI18n data-* fallback suggestion (line 259) is intentionally out of scope per the author's in-thread reply ("fixed at the loader, not papered over per-component"); no change on changed lines.

Files Reviewed (incremental)

• assets/js/components/spark-cart-drawer.js - 0 new issues
• tests/js/skip-to-content.test.js - 0 new issues

Notes

• _announceRaf lifecycle is correct: the outer rAF stores its id, the outer callback synchronously reassigns _announceRaf to the inner id before yielding, so _cancelAnnounce() properly cancels whichever frame is pending.
• New tag-agnostic regex correctly catches both <main id="main-content" tabindex="-1"> and <div id="main-content" tabindex="-1"> (account-only).
• No new findings on changed lines; previous findings all addressed.

Previous Review Summary (commit 8c1456d)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 8c1456d)

Status: 4 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	2
SUGGESTION	2

Issue Details (click to expand)

WARNING

File	Line	Issue
assets/js/components/spark-cart-drawer.js	514	setTimeout(..., 0) deferral can still coalesce in many browsers; rapid identical announcements may not be re-announced by AT. Cleanup in disconnectedCallback only clears the pending timer.
tests/js/skip-to-content.test.js	68	Template scanner only matches <main> elements; account_only uses <div id="main-content">. Two-pronged coverage is fragile for future template variants.

SUGGESTION

File	Line	Issue
assets/js/components/spark-cart-drawer.js	257	window.SparkI18n is read without a data-* fallback; the announcement test never sets SparkI18n so i18n regressions would not be caught.
tests/js/skip-to-content.test.js	91	Drift guard never asserts the contrast fix (color/background) that drove the patch-6 change; a regression swapping #1E293B for a mid-luminance color would pass.

Files Reviewed (22 files)

• assets/js/components/spark-cart-drawer.js - 2 issues
• assets/main.css - 0 issues (compiled artifact)
• css/input.css - 0 issues
• layouts/base.html - 0 issues
• partials/account_only.html - 0 issues
• templates/index.html - 0 issues
• tests/js/skip-to-content.test.js - 2 issues
• tests/js/spark-cart-drawer-announce.test.js - 0 issues
• tests/js/spark-cart-client.test.js - 0 issues (characterization tests for pre-existing behavior are in-scope as test code)
• tests/js/spark-cart-drawer.test.js - 0 issues
• tests/js/spark-cart-rewards.test.js - 0 issues
• tests/js/spark-progress-bar.test.js - 0 issues
• locales/*.json (11 files) - 0 issues

Notes

• Phase-1 "characterization" tests intentionally document pre-existing bugs in unchanged source (quantity 0 promoted to 1, MAX_QTY_PER_LINE not enforced, 429/500 status ignored, duplicate gift line lookup). These are explicit in test comments and acknowledged in the PR description as out-of-scope follow-ups. Not flagged as PR issues.
• The PR's three slices (tests / skip-link / aria-live) are well-scoped, source/CSS rebuilds are consistent, and the locale coverage (11 locales) matches the claim.
• The SparkI18n wiring is plausibly handled by the unchanged spark-cart-loader.js / spark-platform.js populating it from SparkCartAssets (which the PR correctly extends with the two new keys). Flagged as SUGGESTION because the announcement test does not exercise that path.

Fix these issues in Kilo Cloud

---

_{Reviewed by minimax-m3 · Input: 51.9K · Output: 6.2K · Cached: 423.3K}