test(node): Capture V1 SDK compliance harness job#4109
Open
eli-r-ph wants to merge 1 commit into
Open
Conversation
Contributor
📝 No Changeset FoundThis PR doesn't include a changeset. A changeset is required to release a new version. How to add a changesetRun this command and follow the prompts: pnpm changesetRemember: Never use |
Contributor
Contributor
|
Size Change: 0 B Total Size: 17.2 MB ℹ️ View Unchanged
|
eli-r-ph
force-pushed
the
capture-v1/04-node-wiring
branch
from
985fa04 to
4a6fb05
Compare
eli-r-ph
force-pushed
the
capture-v1/05-node-harness
branch
from
178e671 to
082e7f6
Compare
eli-r-ph
force-pushed
the
capture-v1/04-node-wiring
branch
from
4a6fb05 to
b7a98be
Compare
eli-r-ph
force-pushed
the
capture-v1/05-node-harness
branch
2 times, most recently
from
55f9e90 to
57fce26
Compare
eli-r-ph
force-pushed
the
capture-v1/04-node-wiring
branch
from
b7a98be to
3ae7401
Compare
eli-r-ph
force-pushed
the
capture-v1/05-node-harness
branch
from
57fce26 to
651ce50
Compare
eli-r-ph
force-pushed
the
capture-v1/04-node-wiring
branch
from
3ae7401 to
4b7c604
Compare
eli-r-ph
force-pushed
the
capture-v1/05-node-harness
branch
from
651ce50 to
5df130e
Compare
Wire the Capture V1 contract (POST /i/v1/analytics/events) into the node SDK compliance rig, alongside the existing v0 job (dual for now). - adapter: read POSTHOG_CAPTURE_MODE, advertise capture_v1 vs capture_v0 accordingly, and pass captureMode to the client. Thread through historical_migration, translate harness EventOptions into the SDK's sentinel properties, and shorten the v1 retry backoff so retry tests stay within their wait windows. - Dockerfile.v1: bakes POSTHOG_CAPTURE_MODE=v1 (the reusable compliance workflow only passes PORT at runtime). - workflow: add a second job building Dockerfile.v1 with its own report. - compose/README: document POSTHOG_CAPTURE_MODE=v1 for local runs. Verified locally against harness 0.10.0: 110/111 v1 contract tests pass (the one failure is a real Retry-After gap in the partial-retry path, fixed in the transport commit).
eli-r-ph
force-pushed
the
capture-v1/04-node-wiring
branch
from
4b7c604 to
1354e39
Compare
eli-r-ph
force-pushed
the
capture-v1/05-node-harness
branch
from
5df130e to
c3beac4
Compare
![mendral-app[bot]](https://avatars.githubusercontent.com/in/1975249?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Supply Chain Security Review
Tag @mendral-app with feedback or questions. View session
| @@ -0,0 +1,41 @@ | |||
| FROM node:24-alpine | |||
Contributor
There was a problem hiding this comment.
maintainability (P2): Base image node:24-alpine uses a floating tag. Pin to a digest to ensure reproducible builds and prevent silent image substitution.
Suggested change
Suggested change
| FROM node:24-alpine | |
| FROM node:24-alpine@sha256:YOUR_DIGEST_HERE |
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At compliance/node/Dockerfile.v1, line 1:
<issue>
Base image `node:24-alpine` uses a floating tag. Pin to a digest to ensure reproducible builds and prevent silent image substitution.
</issue>
| COPY compliance/node/adapter.js ./ | ||
|
|
||
| # Install adapter dependencies (express) in clean directory | ||
| RUN npm init -y && npm install express |
Contributor
There was a problem hiding this comment.
maintainability (P3), medium confidence: npm install express installs whatever latest version is available at build time. Pin the version for reproducibility (e.g. express@4.21.2).
Suggested change
Suggested change
| RUN npm init -y && npm install express | |
| RUN npm init -y && npm install express@4.21.2 |
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At compliance/node/Dockerfile.v1, line 30:
<issue>
`npm install express` installs whatever latest version is available at build time. Pin the version for reproducibility (e.g. `express@4.21.2`).
</issue>
Contributor
|
Reviews (1): Last reviewed commit: "test(node): add Capture V1 SDK complianc..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (options && typeof options === 'object') { | ||
| for (const [optionKey, sentinel] of Object.entries(OPTION_SENTINELS)) { | ||
| if (Object.prototype.hasOwnProperty.call(options, optionKey)) { | ||
| mergedProperties[sentinel] = options[optionKey] |
Contributor
There was a problem hiding this comment.
When /capture receives an options value with the right key but the wrong runtime type, this line copies it into the SDK sentinel property and the V1 transform then strips the sentinel while omitting the option. For example, a non-string product_tour_id is removed instead of being returned as an adapter validation error, so the harness can send an accepted request whose outbound V1 event silently loses the requested option.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Final PR in the stack. With v1 wired into the client (PR4), this proves the wire behavior against the real contract by running the SDK test harness V1 suite (
POST /i/v1/analytics/events) in CI, alongside the existing v0 job.Changes
compliance/node/adapter.js): readsPOSTHOG_CAPTURE_MODE, advertisescapture_v1vscapture_v0accordingly (so the harness selects exactly one capture contract per process), and passescaptureModeto the client. Also threadshistorical_migrationthrough/init, translates the harnessEventOptionsinto the SDK's sentinel properties (cookieless_mode→$cookieless_mode,disable_skew_correction→$ignore_sent_at,process_person_profile→$process_person_profile,product_tour_id→$product_tour_id), and shortens the v1 retry backoff so retry tests stay within their wait windows.Dockerfile.v1: bakesENV POSTHOG_CAPTURE_MODE=v1— the reusable compliance workflow only passesPORTat runtime, so mode has to be baked into the image.test-node-sdk-v1) buildingDockerfile.v1with its own report name, kept alongside the v0 job while both modes ship dual.POSTHOG_CAPTURE_MODE=v1 docker-compose up …for local runs.Why a separate image/job instead of one adapter serving both
A single adapter process advertises fixed capabilities and can't satisfy both the v0 (
/batch/) and v1 (/i/v1/analytics/events) contracts at once, and the reusable workflow doesn't pass adapter env at runtime. Baking the mode per image and running two jobs is the clean split;capture_tests.yamlrequirescapture_v0andcapture_analytics_v1_tests.yamlrequirescapture_v1, so each run picks up only its contract (the sharedfeature_flagssuite runs in both).Verification
Ran the harness (v0.10.0) locally against the v1 adapter:
The single failure —
partial_batch_handling.respects_retry_after_on_partial— was a real SDK bug: the 200 partial-retry path ignored aRetry-Afterheader on the 2xx response. Fixed in the transport commit on the PR3 branch (fix(node): honor Retry-After on 200 partial-retry responses, + unit test). With that fix the full v1 contract passes, including endpoint/method, all required headers, event format +optionslifting,$set/$set_once/$groupspassthrough, batch envelope, retryable-vs-terminal status matrix, partial-batch pruning, uuid/timestamp/request-id preservation across retries, backoff +Retry-After, compression,historical_migration,$geoip_disable, and the feature-flag side-effect suite.Release info Sub-libraries affected
Libraries affected
No changeset (CI/test-only; the feature changeset is in PR4).
Checklist
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored with Cursor (Claude Opus 4.8 agent). The dual-job approach (keep v0, add v1) follows the maintainer's "run both modes for smoke testing" direction. The local harness run caught a genuine
Retry-Aftergap that unit tests had missed, which is exactly why the harness job earns its place.