Skip to content

Isolate Sparkle smoke launch reports per run - #2218

Merged
r3dbars merged 1 commit into
mainfrom
claude/issue-2157
Oct 18, 2026
Merged

r3dbars merged 1 commit into
mainfrom
claude/issue-2157

Conversation

@r3dbars

@r3dbars r3dbars commented Oct 17, 2026

Copy link
Copy Markdown
Owner

Two overlapping sparkle-update-smoke runs on the same --output shared <output>/<state>/launch-ui-smoke.json. One run could delete the other's report before it was read, or pick up the other's report as its own evidence.

What changed

  • Isolate the Sparkle smoke report between concurrent runs (Sweep fix: Sparkle update smoke ignores a stale launch report #2107 review comment): runScenario now takes the run's runID (already a UUID per run) and puts each scenario's launch-ui-smoke.json, app.log and isolated home under <output>/runs/<runID>/<state>/. Overlapping runs no longer share any of those files.
  • The existing stale-report test now targets the per-run path.
  • New test testConcurrentRunsKeepSeparateLaunchReports: two runIDs on one output, run A passes, run B (a failing fake app) fails, the report paths differ, and run A's report still exists afterwards.
  • Tools/TranscriptedQA/AGENTS.md notes the new layout.

Still shared at the output root on purpose: fake-appcast.xml (evidence only, not read by the scenarios, atomic write) and the summary sparkle-update-smoke.json (atomic write, carries its runID, last run wins). Old per-run folders are kept as evidence and aren't pruned, since pruning could delete a run that's still going.

Checks

  • swift test --package-path Tools/TranscriptedQA --filter SparkleUpdateSmokeTests: pass (6 tests, 0 failures)
  • Regression proof: put the old shared <output>/<state> path back temporarily and ran the new test: it failed on both the path-equality and "run A's report still exists" asserts. Restored.
  • python3 scripts/dev/check-source-pins.py --changed-only: pass
  • python3 scripts/dev/check-test-shape.py: pass
  • bash check.sh: pass (Deterministic proof PASS, 95 TranscriptedQA tests, doc paths OK)
  • Not run: bash run-tests.sh and verify-change.sh (no app-target code changed; this is QA tooling only). The real-app smoke itself wasn't run.

Review

Independent review by Opus over the Sonnet diff, against origin/main (branch was already current). Verdict: correct and complete for the one checklist item. No privacy, audio, main-thread or product-surface impact. Harness safety is better than before: the stale-report delete now only ever runs inside a fresh UUID folder. No checks weakened, no baselines grown, the test doesn't read Sources/ as text, and it's a real regression test (fails on the old layout). No changes needed on top of Sonnet's commit.

Labeled needs owner review because it touches the Sparkle smoke tooling.

Fixes #2157

🤖 Generated with Claude Code

Each sparkle-update-smoke run now writes its launch report, app log and isolated home under <output>/runs/<runID>/<state>, so two overlapping runs on the same output can no longer delete or consume each other's report. Adds a test with two runs sharing one output.

Fixes #2157
@r3dbars r3dbars added the needs owner review Owner must review before merge label Oct 17, 2026
@r3dbars
r3dbars merged commit 221e4ff into main Oct 18, 2026
11 checks passed
@r3dbars
r3dbars deleted the claude/issue-2157 branch October 18, 2026 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs owner review Owner must review before merge risk:high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Follow-up from #2107 review: isolate the Sparkle smoke report between concurrent runs

1 participant