Skip to content

fix(start-session-recording): require replay setup crash containment - #55

Draft
posthog-local-dev[bot] wants to merge 3 commits into
mainfrom
posthog-self-driving/specsession-replay-require-replay-setup-1b83e9
Draft

fix(start-session-recording): require replay setup crash containment#55
posthog-local-dev[bot] wants to merge 3 commits into
mainfrom
posthog-self-driving/specsession-replay-require-replay-setup-1b83e9

Conversation

@posthog-local-dev

Copy link
Copy Markdown

Problem

  • A Teams-plan customer upgraded posthog-android and posthog-ios together and hit a host-app crash on both platforms during session replay setup, before the first masked frame. Both mobile teams are blocked and have rolled back.
  • The no-crash contract lives only in prose. start-session-recording says the call "should not throw" and unavailable integrations "no-op or log"; session-replay-privacy says capture must "fail closed ... rather than crash the app".
  • No acceptance scenario tested it, so nothing forced Android and iOS to converge on crash-containment when replay setup fails.

Changes

  • New canonical requirement on start-session-recording: when the replay configuration is invalid, or the replay integration is unavailable or fails to initialize, the SDK MUST no-op or log and MUST NOT throw into host application code. Recording stays inactive; promise variants resolve rather than reject.
  • Public acceptance coverage for both failure inputs (invalid config, unavailable integration), asserting the call does not throw and recording stays inactive.
  • Tie the start-session-recording Error handling prose to the new requirement.

The non-negotiable outcome tested is "does not throw" + "recording stays inactive". Logging stays optional to match the canonical "no-op or log" wording and not over-constrain SDKs that legitimately stay silent.

Scope

  • Spec-only change, following the OpenSpec propose → apply → archive flow on one branch (change archived under openspec/changes/archive/2026-09-01-require-replay-start-crash-containment/, canonical spec synced).
  • The direct crash fixes still land in posthog-android and posthog-ios. This closes the spec gap so both SDKs are held to the same bar.

Agent context

  • Could not run openspec validate --specs --strict: the OpenSpec CLI is not installed and is not the public npm openspec package. Format was matched by hand against the most recent archived changes and canonical specs.

Created with PostHog Desktop from this inbox report.

Add a canonical requirement and public acceptance scenarios that hold every
replay-capable SDK to the same crash-containment bar. When the replay
configuration is invalid, or the replay integration is unavailable or fails to
initialize, start-session-recording must no-op or log and must not throw into
host application code, and session recording must stay inactive.

The contract existed only in prose and no scenario tested it. The direct crash
fixes land in posthog-android and posthog-ios.

Generated-By: PostHog Desktop
Task-Id: 0422fe0c-2fe9-4a36-91a4-60bf2b7cd40f
@posthog-local-dev

posthog-local-dev Bot commented Sep 2, 2026

Copy link
Copy Markdown
Author

🦔 PostHog Review reviewed this pull request

Found 0 must fix, 2 should fix, 0 consider.

Published 2 findings (view the review).

Resolved comments: 2 fixed

@posthog-local-dev

Copy link
Copy Markdown
Author

PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

@posthog-local-dev posthog-local-dev Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review

Found 2 should fix.

Comment thread openspec/specs/start-session-recording/spec.md
The crash-containment requirement names three failure inputs — invalid
configuration, unavailable integration, and an integration that fails to
initialize — but only the first two had scenarios. An SDK that validates the
configuration before calling the replay initializer, or that never builds the
integration, passes both existing scenarios while still throwing from an
initializer that fails at runtime (native library load failure, missing OS API,
permission refusal) — the exact path the audited crash report describes.

Add a third scenario where replay is configured and eligible to start but the
integration fails to initialize, so the initializer is actually reached, and
assert the untested promise clause alongside "does not throw" and "recording
stays inactive". Mirrored into the public acceptance feature and the archived
change delta.

Generated-By: PostHog Code
Task-Id: 9f7f8b74-ca9d-4eb2-960a-4aece6bc51fc
Tasks 2.1 and 2.3 claimed strict validation without saying what ran, and the
PR description said the OpenSpec CLI was unavailable — so the archived record
was unsupported.

The CLI is available: it is @fission-ai/openspec, the package the repo's own
validation workflow installs. Strict validation now genuinely passes —
`validate --all --strict --no-interactive` reports 62/62 specs, and the change
itself passes `validate --changes --strict` when its archived directory is
staged back as an active change in a scratch copy. Note both commands next to
the tasks so the archive records how the boxes were earned rather than only
that they were checked.

Generated-By: PostHog Code
Task-Id: 9f7f8b74-ca9d-4eb2-960a-4aece6bc51fc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants