Commit Graph
5 Commits
Author SHA1 Message Date
shadowdaoandClaude Opus 5 17540c75b0 test: assert the 2x-budget ceiling the watchdog now guarantees, not 4s
Build / macOS (macos-latest) (push) Successful in 53s
Build / Linux (ubuntu-24.04) (push) Successful in 1m23s
Build / macOS (macos-latest) (pull_request) Successful in 46s
Build / Linux (ubuntu-24.04) (pull_request) Successful in 1m19s
Build / Windows (windows-latest) (push) Successful in 3m57s
Build / Windows (windows-latest) (pull_request) Successful in 3m59s
The Windows job went green, but CTest prints test output only on failure, so a
pass says nothing about WHAT cancelled the request. Under the old 4000ms bound
a pass is ambiguous: the watchdog firing at ~1400ms and WinHTTP's own erratic
cancellation (measured at 1490-4506ms for this same 700ms budget) both fit
under it.

So assert the guarantee the code actually makes now -- a hard ceiling of twice
the caller's budget -- at 2500ms, which is 1400ms plus slack for a loaded
runner. If the watchdog stops doing the work, roughly half the attempts land
above this and print their elapsed time and error string, instead of quietly
passing.

Linux is unaffected: curl honours the 700ms budget exactly, 5/5 at ~701ms.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AzGnvQ6wfD7bw7PZN35ft9
2026-09-09 18:43:32 -07:00
shadowdaoandClaude Opus 5 8888e57d08 fix(http): bound the WinHTTP response-header wait, and stop docs triggering builds
Build / macOS (macos-latest) (push) Successful in 50s
Build / Linux (ubuntu-24.04) (push) Successful in 1m14s
Build / macOS (macos-latest) (pull_request) Successful in 49s
Build / Linux (ubuntu-24.04) (pull_request) Successful in 1m16s
Build / Windows (windows-latest) (push) Failing after 3m55s
Build / Windows (windows-latest) (pull_request) Failing after 3m51s
Two things, both prompted by an intermittent Windows CI failure in
test_api_client's testPlatformBackendTimeout: roughly 2 runs in 6, both its
assertions failed together, meaning a request with timeout_ms=700 waited out a
5s server stall and returned 200. Same failure on 2026-09-07 (job 5834) and
2026-09-09 (job 5911), on code that passed on other runs -- pre-existing and
intermittent, not caused by a change.

1. The real bug. `WinHttpSetTimeouts`' receive parameter maps to
   WINHTTP_OPTION_RECEIVE_TIMEOUT, which Microsoft documents as a PER-PACKET
   Winsock-layer read timeout ("applies to fetching each packet of data off
   the socket"). The wait for the response HEADERS is a separate option,
   WINHTTP_OPTION_RECEIVE_RESPONSE_TIMEOUT, which WinHttpSetTimeouts does not
   set and which defaults to 90 SECONDS. So a server that accepts, reads the
   request and then stalls could block the calling thread for a minute and a
   half no matter what the caller passed as timeout_ms -- precisely the
   "blocking an OBS thread indefinitely" failure that test exists to prevent.
   Now set explicitly, guarded by #ifdef so an older SDK still builds.

   That is a genuine defect on its own merits. Whether it is the whole
   explanation for the intermittency is NOT established: the same docs say
   this timeout "is checked only when data is received from the socket", so
   neither option guarantees a hard deadline -- that needs a watchdog calling
   WinHttpCloseHandle, deliberately not done here.

2. Evidence, so the next run says more than pass/fail. The probe now runs 5
   times and prints elapsed ms, ok, status, requests_seen and the backend's
   error string (carrying GetLastError) for every attempt, so one CI run
   yields a failure RATE and an error code. Each attempt gets a FRESH
   loopback server: the server handles one connection at a time on a single
   thread, so reusing it would leave attempts 2..n in the accept backlog --
   never accepted, a different scenario from the one that fails. Verified on
   Linux: 5/5 attempts give up at ~701ms.

Also: build.yml now has paths-ignore for **.md, LICENSE, NOTICE, and the two
release-only files. This is a full three-platform build behind a runner with
capacity:1, and six of them fired for one afternoon of documentation edits.
Nothing that feeds a build or a test is on that list. Tradeoff: a docs-only
push now shows no status at all rather than a green one.

The WinHTTP change cannot be compiled locally (Linux host); CI is its first
build. All 6 suites pass locally on Linux.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AzGnvQ6wfD7bw7PZN35ft9
2026-09-09 18:30:02 -07:00
shadowdaoandClaude Sonnet 5 969b8db94a license: relicense first-party code from GPL-2.0-or-later to Apache-2.0
Build / macOS (macos-latest) (push) Successful in 33s
Build / Linux (ubuntu-24.04) (push) Successful in 54s
Build / Windows (windows-latest) (push) Successful in 12m10s
Owner sign-off: replace root LICENSE with Apache License 2.0, add a root
NOTICE file, and swap the GPL-2.0 boilerplate header in every first-party
core/ and obs-adapter/ source file for a short Apache-2.0 notice.

This resolves review finding C2 (GPLv2 top-level LICENSE vs. the vendored
Apache-2.0 LiveKit SDK is a license-compatibility violation): the whole
repo is now Apache-2.0, matching LiveKit, so there's no GPL/Apache clash
left. Updated the README Status gate and the CI workflow comment to reflect
that C2 is resolved, while leaving the C1 WebRTC/OpenH264 patent/royalty
gate untouched -- that question is still open and still blocks release.

third_party/ stays under its own upstream licenses; only this project's own
code changed hands. All 6 CTest suites still pass after the header swap.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RL8abRmgFXkVASHkkqiJbE
2026-09-07 04:44:16 -07:00
shadowdaoandClaude Sonnet 5 f2a4932eea Fix review findings: stopping-flag race, unpinned SDK download, key-leak via redirect/logs
Code review findings from before merging feat/livekit-integration to main:

- I1: sourceDestroy set self->stopping outside self->mutex, then notified.
  The worker's condition-variable predicate reads `stopping` under that same
  mutex, so the store+notify could land between the worker's predicate check
  and it entering the wait, dropping the notification and leaving the worker
  asleep for its full backoff (up to 30s) with the OBS UI thread blocked in
  worker.join(). Now set under the lock, matching how `generation` is
  already mutated in applySettings.

- I3: the LiveKit SDK archive download in cmake/LiveKitSDK.cmake had no
  SHA256 pin wired up from the top-level CMakeLists.txt, unlike the obs-deps
  bootstrap right next to it. Added real SHA256 hashes -- computed by
  downloading each release archive and running sha256sum -- for every
  triple the pinned v1.10.1 release can resolve to (Linux x64/arm64, macOS
  x64/arm64, Windows x64), keyed by version+triple so a future version bump
  fails loudly (via message(WARNING)) instead of silently going unverified.
  Verified end-to-end locally: a deliberately wrong hash makes the configure
  step fail with a HASH mismatch error. Only Linux was also build-tested in
  this environment; macOS/Windows archives were downloaded and hashed but
  not build-tested here.

- I4: the curl HTTP backend followed up to 3 redirects while the read key
  travels as a URL query parameter, so a malicious/misconfigured redirect
  (including an HTTPS->HTTP downgrade, which curl doesn't refuse by default)
  could leak the key. This client only ever talks to two fixed, first-party
  endpoints, so redirects are disabled outright (CURLOPT_FOLLOWLOCATION 0),
  matching the WinHTTP backend's existing default behavior. Left
  normalizeServerUrl's explicit-http:// pass-through as-is with a comment,
  per review guidance.

- I5: ApiClient::redactedUrl was tested but never called. No current call
  site logs a request URL, so rather than inventing one, added a one-line
  comment marking it a deliberate guard rail for future logging.

- I7: the LiveKit SDK log bridge (livekitLogToObs) wrote SDK messages
  straight into the OBS log. LiveKit's signaling URL carries the access
  token as a query parameter; defensively scrub "access_token=" and "key="
  values before they ever reach obs_log. New ApiClient::redactSensitiveParams
  generalizes redactedUrl's redaction pattern to arbitrary text (not just a
  bare URL), with 6 new unit tests in test_api_client.cpp.

- I2: added a code comment on session.cpp's auto_subscribe=true noting the
  known, unaddressed bandwidth/CPU cost of pulling every participant's
  track in multi-camera rooms, and that per-publication unsubscribe is a
  future optimization. No behavior change (out of scope per review).

Verified: cmake configure + build + `ctest --test-dir build
--output-on-failure` all pass, 6/6 suites (test_api_client now 127 checks,
up from 121).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RL8abRmgFXkVASHkkqiJbE
2026-09-06 22:59:57 -07:00
shadowdaoandClaude Sonnet 5 bf33966a4e Add the streamer-tools API client, with a real HTTP backend per platform
Implements the two read-key-scoped calls in
apps/server/src/obs/plugin.routes.ts: GET /api/obs/:slug/slots and
POST /api/obs/:slug/token.

Three pieces, all in core/ with no OBS dependency:

- stplugin::json -- a small, strict JSON reader. Hand-rolled rather than
  vendoring nlohmann because the only JSON this plugin ever sees is two
  fixed-shape responses from its own server, and the parser has to build on
  three platforms with no package-manager step in CI. It never throws,
  bounds its recursion (kMaxDepth=32) so a hostile response cannot overflow
  the stack inside OBS, rejects trailing garbage, and returns the caller's
  fallback for wrong-typed access instead of aborting.

- stplugin::HttpClient -- a two-method injectable interface, with libcurl
  behind it on Linux/macOS and WinHTTP on Windows. WinHTTP rather than curl
  on Windows because it ships with the OS and does TLS through SChannel: the
  self-hosted winvm-builder runner has no package manager, and per the
  scaffold README does not even have cmake preinstalled. Both backends cap
  the response body at 4 MiB, keep TLS verification on (the read key is a
  credential), and honour a whole-request timeout.

- stplugin::ApiClient -- maps the responses onto an ApiStatus enum that
  distinguishes NotFound (404), Unavailable (503), NetworkError,
  MalformedResponse and InvalidConfig. It deliberately does not claim to
  know whether a 404 was a wrong key or an unknown slug, because the server
  deliberately does not say. Server URLs are normalised the way an operator
  actually pastes them, defaulting to https so the read key is never sent in
  the clear by accident, and redactedUrl() exists so a URL can be logged
  without the key.

Tests (279 checks across two new suites) run at two levels: a fake
HttpClient covering every response and error branch, and a real loopback
HTTP server on 127.0.0.1 driving the actual platform backend -- so libcurl
on Linux/macOS and WinHTTP on Windows are each exercised in CI rather than
assumed. The loopback cases deliberately include the ones that must not hang
OBS: a truncated JSON body, a connection accepted and closed without a
reply, non-HTTP garbage, a dead port, and a stalled server that has to be
cut off by the client's own timeout.

Verified locally on Ubuntu 24.04:
  ctest --test-dir build --output-on-failure -> 4/4 passed
  test_json: 158 checks passed
  test_api_client: 121 checks passed

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RL8abRmgFXkVASHkkqiJbE
2026-09-06 21:28:55 -07:00