fix/mrr-base

needs attentionviewing older commit

1a8164f · incrementalpre-PRreviewed 2026-08-11 22:46 UTC2H · 2M · 4L · 5I

The branch

Purpose

Harden the PostHog deploy annotation system so the gate and the annotation text never re-enumerate behavioral lanes independently.

Goal

Single-source the deploy lane computation in a new resolve-deploy-lanes job, making adding a new lane a one-line change rather than a multi-site edit.

Sub-goals

Introduce resolve-deploy-lanes job that emits any (gate) and lanes (display text) outputs
Update posthog-annotate job to consume those outputs instead of duplicating lane logic
Update posthog-annotations.md rule to reflect the new single-source convention
Remove whitespace normalization from posthog-annotate.mjs (now handled upstream)

The changes (whole branch)

What: New resolve-deploy-lanes job added to release-promote.yml; posthog-annotate job now depends on it for its gate condition and lane display text; posthog-annotate.mjs .replace() whitespace normalization removed; posthog-annotations.md updated to document the one-line change invariant.
Why: Previous implementation gated annotation on vercel-promote success and constructed the lanes string in two separate places (workflow condition + annotation script), creating a maintenance trap where adding a lane required edits in multiple spots and risked inconsistency.
Areas: .claude/rules/posthog-annotations.md+5−3.github/workflows/release-promote.yml+50−18scripts/posthog-annotate.mjs+10−7
Blast: PostHog annotation system only — no app code, no DB, no API, no CDK. A failure in the resolver silently suppresses annotations (no customer impact) but degrades observability of future prod deploys.

stripe-bootstrap excluded from behavioral lanes without rationale continue-on-error on resolver (no side effects) can mask failures silently buildContent() untested after modification bash resolver logic has no test harness

➖CI· No PR open➖CodeRabbit· Not wired in this repo✅correctness· Refactor is functionally correct; GHA output wiring handles empty/false/true strings correctly; dependency chain preserves ordering✅security· No injection risk; lanes are hardcoded; dispatch-only gate prevents fork poisoning; permissions correctly minimized❌conventions· stripe-bootstrap exclusion undocumented; continue-on-error scope too wide on resolver❌tests· buildContent() and bash resolver logic — the two changed pieces — are entirely untested✅improvement· Clean DRY refactor; minor cosmetic options only➖seo· Self-skipped — no apps/web/ files in diff

Findings · 8

correctness1

low

posthog-annotate reduced needs: list obscures behavioral lane dependencies in GHA graph

.github/workflows/release-promote.yml

Functionally correct — ordering is preserved transitively through resolve-deploy-lanes. Cosmetically, the GHA dependency graph no longer shows all behavioral lanes as explicit prerequisites of posthog-annotate. A comment clarifying the transitive ordering would aid future readers.

conventions2

medium

stripe-bootstrap excluded from behavioral lanes without documented rationale

.github/workflows/release-promote.yml

stripe-bootstrap is the only lane omitted from add() calls with no explanation. If it is a one-time setup lane, that should be stated in a comment. If it is a recurring behavioral lane, a stripe-bootstrap-only promote would silently fire no annotation. Recommended: add an inline comment explaining the exclusion, or include it in the lane list.

medium

continue-on-error on resolve-deploy-lanes can silently suppress annotations

.github/workflows/release-promote.yml

continue-on-error is correct on posthog-annotate (failure must never block a release). But resolve-deploy-lanes has no side effects — if it fails, outputs are empty, any is falsy, and the annotation is skipped with no visible failure signal. Recommended: remove continue-on-error from resolve-deploy-lanes; keep it only on posthog-annotate.

tests3

high

buildContent() changed, untested, and unexportable without refactor

scripts/posthog-annotate.mjs

The function changed in this commit (.replace() removed) and drives the annotation message format. It is not exported and has no test coverage. Established pattern is a -core extraction. A regression silently corrupts annotation text in prod with no CI signal. Recommended: extract to an exportable pure function and add tests covering happy path and edge cases (missing actor, empty lanes).

high

resolve-deploy-lanes bash logic has no test harness

.github/workflows/release-promote.yml

The inline bash in resolve-deploy-lanes is the new gate for whether an annotation fires at all. No shellcheck run, no bats suite, no matrix test exists. A whitespace or quoting regression silently suppresses every future prod annotation. Recommended: add shellcheck to CI for embedded bash, or extract to scripts/resolve-deploy-lanes.sh and add a bats test for the add() and output logic.

medium

No CI gate validates lanes output format consumed by annotation script

.github/workflows/release-promote.yml

The lanes output from resolve-deploy-lanes is consumed verbatim by posthog-annotate.mjs. A format drift (e.g. extra whitespace, different delimiter) would be caught only at runtime in production. A contract test or a CI smoke assertion over the output format would close this gap.

improvement2

low

any output could be simplified to lanes != '' at callsite

.github/workflows/release-promote.yml

The any boolean output is defensively explicit and reasonable to keep, but the callsite condition `== 'true'` could equivalently check lanes != ''. Minor cosmetic simplification only.

low

|| true in add() is slightly non-idiomatic bash

.github/workflows/release-promote.yml

|| true works correctly but || : or a [[ ]] && pattern is more idiomatic in bash. No functional impact.

History · 4 commits

2db5192needs attentionincremental0H · 4M · 5L2026-08-11 22:54
1a8164fneeds attentionincremental2H · 2M · 4L2026-08-11 22:46current
e807453needs attentionincremental0H · 2M · 3L2026-08-11 19:07
69a0691needs attentionfull3H · 7M · 5L2026-08-11 18:12