chore: remove downstream-consumer + personal refs (OSS hygiene) - #3882
Merged
Conversation
Make the public stack consumer-agnostic. No real behaviour change: - posthog-context middleware: CLI user-agent detection is now config-driven via config.analytics.cliUserAgentPattern (regex-source string). Empty/unset default -> no CLI detection (source stays 'web'). Removes the hardcoded downstream CLI user-agent literal. - config helper: genericize the known-weak JWT secret reject-list to generic placeholders (ExampleNodeDevSecret / ChangeThisDevSecret). - genericize downstream-named examples + incident provenance in code comments, config, docs (ERRORS.md, MIGRATIONS.md, README.md, RUNBOOKS.md, module READMEs, migration doc, update-stack skill) and tests. Tests + lint green; coverage unchanged (gated server-side via Codecov).
|
Caution Review failedPull request was closed or merged during review WalkthroughReplaces the hardcoded ChangesConfig-driven CLI UA detection and JWT placeholder generalization
Trawl reference de-branding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3882 +/- ##
==========================================
- Coverage 92.67% 92.66% -0.02%
==========================================
Files 169 169
Lines 5529 5535 +6
Branches 1776 1778 +2
==========================================
+ Hits 5124 5129 +5
- Misses 325 326 +1
Partials 80 80
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR removes downstream-consumer and personal references across the stack for OSS hygiene, while keeping behavior stable. It also makes PostHog CLI source attribution opt-in via configuration instead of a hardcoded downstream CLI User-Agent pattern.
Changes:
- Make
posthog-context.middleware.jsCLI detection config-driven viaanalytics.cliUserAgentPattern(regex-source string; empty disables detection). - Genericize the JWT weak-secret placeholder denylist in
lib/helpers/config.jsand adjust associated tests. - Scrub downstream/personal references from docs, runbooks, migrations notes, and test fixtures/examples.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Genericize CLI source description + project override examples. |
| modules/invitations/README.md | Remove downstream-specific module example naming. |
| modules/core/tests/core.unit.tests.js | Replace downstream env-name example with a generic one. |
| modules/billing/services/billing.webhook.service.js | Genericize downstream responsibility comment. |
| modules/billing/RUNBOOKS.md | Replace downstream URLs/secret names/CronJob names with placeholders. |
| modules/billing/repositories/billing.subscription.repository.js | Genericize downstream migration comment. |
| modules/billing/README.md | Remove downstream-specific incident provenance; keep guidance generic. |
| modules/billing/middlewares/billing.requirePlan.js | Genericize “downstream contract” wording in comment. |
| modules/billing/middlewares/billing.attachUsageContext.js | Remove downstream-specific example mention in comment. |
| MIGRATIONS.md | Remove downstream repo references and genericize downstream guidance. |
| lib/services/tests/express.openapi-servers.unit.tests.js | Replace downstream domain fixture with generic domain. |
| lib/services/tests/analytics.captureException.unit.tests.js | Replace downstream appTag test fixture with generic one. |
| lib/services/tests/analytics.capture.unit.tests.js | Replace downstream appTag test fixture with generic one. |
| lib/middlewares/tests/posthog-context.middleware.unit.tests.js | Update tests to cover config-driven CLI UA detection and “disabled when unset”. |
| lib/middlewares/posthog-context.middleware.js | Switch hardcoded CLI UA detection to config-driven regex-source string. |
| lib/helpers/tests/config.validateJwtSecret.unit.tests.js | Update placeholder-secret test fixtures to generic/upstream terms. |
| lib/helpers/tests/config.isJwtSecretWeak.unit.tests.js | Update weak-secret placeholder coverage to generic/upstream terms. |
| lib/helpers/config.js | Replace downstream-specific JWT placeholders with generic placeholders. |
| ERRORS.md | Remove downstream repo references while keeping the actionable guidance. |
| docs/migrations/2026-05-01-billing-crons-module-relocation.md | Remove downstream-specific example project mention. |
| config/index.js | Genericize comment describing non-standard NODE_ENV usage. |
| config/defaults/development.config.js | Add analytics.cliUserAgentPattern default (env-configurable, empty disables). |
| config/defaults/billing.pricing.constants.js | Remove downstream-specific provenance from header comment. |
| .claude/skills/update-stack/SKILL.md | Remove downstream-specific plan reference. |
Comment on lines
+15
to
+28
| /** | ||
| * Build the CLI User-Agent matcher from config, or `null` when unconfigured. | ||
| * | ||
| * @returns {RegExp|null} compiled regex, or `null` if no/invalid pattern is set | ||
| */ | ||
| const getCliUaRe = () => { | ||
| const pattern = config.analytics?.cliUserAgentPattern; | ||
| if (!pattern) return null; | ||
| try { | ||
| return new RegExp(pattern); | ||
| } catch { | ||
| return null; | ||
| } | ||
| }; |
Comment on lines
+8
to
12
| * Verifies config-driven User-Agent parsing for CLI vs web source attribution: | ||
| * 1. CLI UA with version → source:'cli', cli_version:'<version>' | ||
| * 2. CLI UA without explicit version segment → source:'cli' fallback | ||
| * 3. Web browser UA → source:'web' | ||
| * 4. Missing UA → source:'web' |
Comment on lines
162
to
166
| ## Test DB isolation: per-pid Mongo database default + globalTeardown (2026-04-24) | ||
|
|
||
| Default test database is now `mongodb://127.0.0.1:27017/NodeTest_${process.pid}` instead of the shared `NodeTest`. Concurrent jest invocations (e.g. multiple agent worktrees running `npm run test:coverage` in parallel) get isolated databases, eliminating the 401 / 404 / 422 / `MongoPoolClosedError` flake patterns documented in trawl_node#980. | ||
| Default test database is now `mongodb://127.0.0.1:27017/NodeTest_${process.pid}` instead of the shared `NodeTest`. Concurrent jest invocations (e.g. multiple agent worktrees running `npm run test:coverage` in parallel) get isolated databases, eliminating the 401 / 404 / 422 / `MongoPoolClosedError` flake patterns seen in parallel runs. | ||
|
|
||
| ### What changed |
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
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.
Public OSS hygiene — the stack must not reference its consumers. Removes ALL downstream-consumer + personal refs (24 files), keeping logic + tests green.
Functional:
posthog-context.middleware.js: CLI user-agent detection is now config-driven (config.analytics.cliUserAgentPattern, regex-source string; default''= no detection). Was a hardcoded downstream-CLI pattern. Consumers opt in via config.lib/helpers/config.js: genericized the downstream-named entries in the weak-dev-secret reject-list.Comments/docs/tests: genericized downstream examples + incident provenance (ERRORS/MIGRATIONS/README/billing-README/RUNBOOKS/invitations-README/skills + test example env-names). Kept all
pierreb-devkitself-identity + the maintainer contact email.Lint clean, 2588 tests pass, coverage held. Final OSS self-grep: only the maintainer email remains (intentional).
Summary by CodeRabbit
New Features
Documentation
Chores