chore/ph-annot
needs attentionviewing older commit5d8dfd6 · fullPR #359reviewed 2026-07-30 17:33 UTC0H · 1M · 5L · 6I- Purpose
- Add PostHog deploy annotations so metric changes on charts are attributable to prod releases at a glance
- Goal
- Wire a non-gating posthog-annotate CI job into the prod release workflow that posts a deploy marker after a successful Vercel promotion
- Sub-goals
- SG-1: New script scripts/posthog-annotate.mjs — resilient, exits 0 on any error
- SG-2: New posthog-annotate job (Phase 5b) in prod release workflow, fires only on gated prod promotions
- SG-3: New SSOT rule .claude/rules/posthog-annotations.md documenting the convention, trigger conditions, and guardrails
- What
- Adds deploy annotation capability to the prod release pipeline. Three new files (230 lines total): the sender script, the CI job wiring, and the SSOT rule.
- Why
- Without deploy markers a metric jump on a PostHog chart is indistinguishable from a behaviour change, a product change, or an instrumentation regression. One marker per prod promotion turns the timeline into a causal narrative.
- Areas
- scripts/posthog-annotate.mjs+107−0.github/workflows/release-promote.yml+42−0.claude/rules/posthog-annotations.md+81−0
- Blast
- 3 files, +230/0. Release pipeline only (non-gating, continue-on-error). No domain logic, no DB, no API surface changes.
Findings · 12
correctness1
No correctness bugs found
Script logic, error handling, and CI job wiring are all correct. The exit-0 guarantee is structurally sound at both the script level and the job level.
security4
posthog-annotate inherits top-level id-token:write — unnecessary
.github/workflows/release-promote.yml
The job inherits the top-level `permissions: { id-token: write, contents: read }`. Annotating PostHog needs only `contents: read`. Adding a job-level `permissions: { contents: read }` block overrides the inherited grant and removes an unnecessary OIDC minting capability from this job.
ANNOTATION_CONTENT printed to runner log verbatim
scripts/posthog-annotate.mjs:97
The success log prints the full body.content string. While the default content only includes SHA/actor/URL (none sensitive), the ANNOTATION_CONTENT env override could embed a value that was not meant to appear in plain-text runner logs. Low risk in practice.
POSTHOG_HOST not validated as HTTPS before sending credentials
scripts/posthog-annotate.mjs:2
If a repo admin misconfigures vars.POSTHOG_HOST to http://, the Bearer token is sent in plaintext. Risk is low (vars is admin-controlled, default is HTTPS), but an early https:// assertion would make the invariant explicit.
Project ID 334265 hardcoded — acceptable per SSOT, minor drift risk
.github/workflows/release-promote.yml
The posthog-annotations.md SSOT explicitly documents 334265 as the prod project. Not sensitive. If the PostHog project is ever recreated the job would silently annotate the wrong project.
conventions2
No skip_annotation input toggle — parity gap with other phases
.github/workflows/release-promote.yml
Every other optional phase has a skip_* workflow_dispatch input (skip_cdk, skip_migrations, skip_stripe, skip_vercel, skip_posthog, skip_tinybird). posthog-annotate has no skip_annotation, so operators cannot suppress the annotation from the dispatch UI without removing the API key secret. Adding skip_annotation: boolean default: false would match the established pattern.
postflight in needs is an ordering dependency not a gate — intentional
.github/workflows/release-promote.yml
The if condition does not check needs.postflight.result, so the annotation fires even when post-flight health check fails. This is by design (YAML comment explains it). Info only so a future maintainer does not add the missing result check.
tests1
No unit tests for buildContent() — consistent with project CI script practice
scripts/posthog-annotate.mjs
The project Vitest scope covers domain decisions and shells, not fire-and-forget infra scripts. No comparable CI script is unit-tested. The conditional format logic is trivial and the output is decorative. No tests required.
improvement4
DEPLOY_URL embeds the rotating Vercel deployment URL, not the canonical prod URL
.github/workflows/release-promote.yml
The annotation embeds `needs.preflight.outputs.production_deployment_url` — the ephemeral Vercel hostname (e.g. `batu-codebase-platform-abc123.vercel.app`). This is exactly what CLAUDE.md warns against: 'copy-pasted links 404' because the URL rotates per deploy. Use the stable `PRODUCTION_URL` secret (already used by the postflight healthcheck) instead, or `VERCEL_PROJECT_PRODUCTION_URL`.
Full checkout + Node setup for a zero-dependency script is heavyweight
.github/workflows/release-promote.yml
The job runs actions/checkout + actions/setup-node (~25s) just to execute a 107-line pure-Node script with no npm dependencies (uses only built-in fetch). Not critical given continue-on-error: true but worth noting.
POSTHOG_HOST var override is redundant — double-default with no documented use case
.github/workflows/release-promote.yml
The workflow passes `vars.POSTHOG_HOST || 'https://us.posthog.com'` and the script also defaults to `https://us.posthog.com` internally. The repo uses the US PostHog cloud with hardcoded project ID 334265. The extra override adds indirection without a known use case.
Success log includes date_marker — useful, no change needed
scripts/posthog-annotate.mjs:97
The success log prints the actual date_marker written to PostHog (not wall-clock time), which is useful for cross-referencing on a chart. Keep it.