fix: preserve initial watch event services and refresh build tooling

[steipete]

watch could label the first AVTransport or RenderingControl notification as unknown when it arrived before the caller recorded the SID from the subscription response. Resolve service names when queued events are consumed, after both subscriptions are established. This removes the shared SID map and preserves existing buffering, unknown-SID behavior, and subscription cleanup.

The JSON regression receives and parses the SUBSCRIBE response, then holds return to the caller while an initial NOTIFY is acknowledged. This deterministically proves the application scheduling race while respecting UPnP response-first ordering. This race surfaced during CI for a tooling refresh, so the PR also prefers Go 1.26.8 without raising the Go 1.26 source minimum, adds minimum-version CI, aligns Docker, and updates golangci-lint, the Dockerfile frontend, and pinned Pages actions. Runtime module versions are already current. Both new macOS architectures retain the released macOS 12.0 minimum.

Validation:

• The deterministic watch regression fails before the fix; all watch command tests pass 20 repetitions with the race detector afterward.
• Actual built CLIs from Go 1.26.0 and 1.26.8 pass synthetic response-first notifications for both services, an unregistered SID, and unsubscribe cleanup. An additional robustness check sends notifications before the response; that case is not a UPnP ordering guarantee.
• Full make ci with Go 1.26.8 passed format, coverage, lint, race tests, and vet; full Go 1.26.0 tests passed. Local Go checks used GOMAXPROCS=2 and GOFLAGS=-p=2.
• Built-CLI checks passed version/help, isolated config persistence and aliases, SOAP topology/volume operations, and JSON/TSV output.
• Release-style macOS builds were compared with published v0.3.4 using vtool; all report macOS 12.0.
• Docker build with fresh base images and smoke checks for sonos, ffmpeg, yt-dlp, and curl passed on the tooling commit; final-head Docker CI verifies the combined candidate.
• Workflow syntax validation and documentation generation passed.

The first PR CI run exposed the watch race while push CI passed. The fix addresses the reproduced ordering defect rather than relying on a retry.

Independent full-candidate P0–P2 review found no actionable issues after the watch fix.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 4, 2026, 7:42 AM ET / 11:42 UTC.

ClawSweeper review

What this changes

The branch defers watch event service labeling until both Sonos subscriptions are established, adds regression coverage, and aligns the Go/tooling, Docker, CI, and related documentation.

Merge readiness

✅ Ready for maintainer review

Keep open for normal maintainer landing: the owner-authored PR has a contained fix for the early watch notification race, a deterministic regression test, and clean current-head checks; no actionable introduced defect was found.

Priority: P2
Reviewed head: 8d18787c2e26254fd7c54444fb7b84943c57d3a1

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A normal good PR: the production change is contained, the race is directly covered, and the supplied current-head checks are clean.
Proof confidence	🌊 off-meta tidepool	Not applicable: Not applicable to this owner-authored PR. The supplied PR body additionally reports built-CLI response-first callback checks, while the introduced test exercises the production watch callback and subscription path deterministically.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: Not applicable to this owner-authored PR. The supplied PR body additionally reports built-CLI response-first callback checks, while the introduced test exercises the production watch callback and subscription path deterministically.
Evidence reviewed	5 items	Deferred service resolution: The callback now queues SID, sequence, timestamp, and parsed variables without reading shared subscription state; after both subscriptions return, the consumer resolves the SID through an immutable two-service map and preserves unknown for unregistered SIDs. Deterministic race coverage: The regression test holds the AVTransport subscription return after its HTTP response is received, sends a callback notification during that interval, releases setup, and asserts JSON output retains the avtransport service. Subscription contract: The Sonos client only returns a Subscription after HTTP.Do returns and validates the response SID, which establishes the caller-side interval modeled by the new test.
Findings	None	None.
Security	None	None.

How this fits together

sonos watch receives UPnP callback notifications from a selected speaker, queues parsed events while subscriptions are being established, then formats them for terminal, JSON, or TSV output. The change moves service-name resolution from the concurrent callback handler to the post-subscription consumer.

flowchart LR
  A[Sonos speaker] --> B[UPnP notification callback]
  B --> C[Buffered event queue]
  D[AVTransport subscription] --> E[Subscription service map]
  F[RenderingControl subscription] --> E
  C --> G[Resolve service after setup]
  E --> G
  G --> H[Terminal, JSON, or TSV output]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Watch race coverage	production +13/-14; regression test +19/-1	The behavioral repair is small and is paired with a targeted scheduling-race test.
Changed surface	10 files affected	The PR couples the watch fix with coordinated CI, Docker, module, changelog, and documentation updates.

Technical review

Best possible solution:

Land the bounded queue-consumer fix together with its deterministic regression coverage and minimum/preferred Go-version checks.

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

Yes—source and the added deterministic harness define a high-confidence path: hold the first subscription return after its response, deliver a callback notification, then verify the queued event is labeled after setup. Target code was not executed during this read-only review.

Is this the best way to solve the issue?

Yes—the change preserves callback buffering and unknown-SID behavior while moving only the race-prone lookup to a point after both subscriptions exist.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against db3a674eaa5b.

Labels

Label changes:

• add P2: This repairs a real but narrow early-event labeling defect in an established command without evidence of broad message loss or availability impact.
• remove P3: Current review triage priority is P2, so this older priority label is no longer current.

Label justifications:

• P2: This repairs a real but narrow early-event labeling defect in an established command without evidence of broad message loss or availability impact.
• 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: Not applicable to this owner-authored PR. The supplied PR body additionally reports built-CLI response-first callback checks, while the introduced test exercises the production watch callback and subscription path deterministically.

Evidence

What I checked:

• Deferred service resolution: The callback now queues SID, sequence, timestamp, and parsed variables without reading shared subscription state; after both subscriptions return, the consumer resolves the SID through an immutable two-service map and preserves unknown for unregistered SIDs. (internal/cli/watch.go:81, 49d08dd87b6b)
• Deterministic race coverage: The regression test holds the AVTransport subscription return after its HTTP response is received, sends a callback notification during that interval, releases setup, and asserts JSON output retains the avtransport service. (internal/cli/watch_cmd_test.go:100, 8d18787c2e26)
• Subscription contract: The Sonos client only returns a Subscription after HTTP.Do returns and validates the response SID, which establishes the caller-side interval modeled by the new test. (internal/sonos/events.go:45, 8d18787c2e26)
• Current-head validation status: The supplied current-head check report records successful test, minimum-go, and docker runs; this also satisfies the preceding review cycle's request for a clean test check. (.github/workflows/ci.yml:56, 8d18787c2e26)
• Watch feature history: Local path history identifies the original watch subscription feature and the current fix as work by Peter Steinberger; raw commit records show the test commit has the fix commit as its recorded parent. (internal/cli/watch.go:131, d0749d90e9af)

Likely related people:

• steipete: 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 (1 earlier review cycle)

• reviewed 2026-09-04T11:31:18.549Z sha df498dc :: blocked before merge. :: none

[steipete]

Merged as b7850a276c8c549532f3fcd88c29dad68facf2b4.

The protocol-compliant regression receives the subscription response before sending the notification and reproduces the old SID-registration race. The fix passed 20 race-detector repetitions and built-CLI response-first checks on Go 1.26.0 and 1.26.8, including unknown-SID handling and unsubscribe cleanup. The earlier response-withholding fixture remains an additional robustness check, not a UPnP ordering guarantee. Fresh full P0–P2 review found no actionable issues.

All workflows passed on the exact merge commit: CI, Docker, Pages, and Dependency Graph. The live watch documentation and install documentation returned HTTP 200, and their HTML matched a fresh render of the merged tree byte for byte.