Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

- Read and follow [CONTRIBUTING.md](./CONTRIBUTING.md) before contributing to the SDK. It covers development setup, the example app, CI-aligned checks, and pull requests.
- Read and follow [RELEASING.md](./RELEASING.md) when adding change intents (`pnpm change`) or working on publishing.
- Before adding or changing public API, follow "Public API changes" in [CONTRIBUTING.md](./CONTRIBUTING.md): the API shape must be agreed on the issue first. For SDK design guidance, read https://posthog.com/handbook/engineering/sdks/guidelines.md.
- Public API changes (a diff in `api/public-api.json`): if the author is a PostHog maintainer (git email ends in `@posthog.com`), the PR is the discussion, so don't open or suggest an issue. Otherwise, follow "Public API changes" in [CONTRIBUTING.md](./CONTRIBUTING.md): if there's no agreed issue, stop and tell the user. If a PR already exists, add a public-API note to its description and draft an issue body for the user to post. Never open an issue yourself. For SDK design guidance, read https://posthog.com/handbook/engineering/sdks/guidelines.md.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Maintainer detection is unreliable

The @posthog.com email check misclassifies Client Libraries maintainers who commit with GitHub noreply or personal addresses. Current maintainers have authored recent commits using @users.noreply.github.com, so agents will still stop their public-API PRs and request duplicate issues—the behavior this change is intended to eliminate. Use actual team membership or another reliable maintainer signal instead of the commit-email domain.

Prompt To Fix With AI
This is a comment left during a code review.
Path: AGENTS.md
Line: 5

Comment:
**Maintainer detection is unreliable**

The `@posthog.com` email check misclassifies Client Libraries maintainers who commit with GitHub noreply or personal addresses. Current maintainers have authored recent commits using `@users.noreply.github.com`, so agents will still stop their public-API PRs and request duplicate issues—the behavior this change is intended to eliminate. Use actual team membership or another reliable maintainer signal instead of the commit-email domain.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Open PR guidance conflicts

The external-contributor instructions first say to stop whenever no agreed issue exists, even when a PR is already open. The next sentence and CONTRIBUTING.md instead say an open PR should continue with a note and drafted issue. An agent handling an existing public-API PR can therefore halt before reaching the intended non-blocking path. Make the stop condition explicitly apply only when no PR exists.

Prompt To Fix With AI
This is a comment left during a code review.
Path: AGENTS.md
Line: 5

Comment:
**Open PR guidance conflicts**

The external-contributor instructions first say to stop whenever no agreed issue exists, even when a PR is already open. The next sentence and `CONTRIBUTING.md` instead say an open PR should continue with a note and drafted issue. An agent handling an existing public-API PR can therefore halt before reaching the intended non-blocking path. Make the stop condition explicitly apply only when no PR exists.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor (non-blocking): Could we treat @posthog.com as one signal of maintainer status rather than the exclusive test? A confirmed Client Libraries maintainer can use a GitHub noreply address, which sends them through the external-contributor issue-first flow despite the exemption in CONTRIBUTING.md. Consider accepting independently confirmed maintainer identity too.

- Keep shared development guidance in `CONTRIBUTING.md` and release guidance in `RELEASING.md` rather than duplicating it here.
7 changes: 4 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ php phpcs.phar --standard=phpcs.xml --extensions=php .

Public API is hard to change once it ships, so agree on it before writing the implementation. Our [SDK guidelines](https://posthog.com/handbook/engineering/sdks/guidelines) explain how we design it.

- If you need something the SDK doesn't support and it would add or change a public option, method, or type, open an issue describing your use case first. At this stage, context is more useful to us than code.
- Wait for a maintainer to agree on the API shape on the issue before implementing it.
This section is for external contributors. PostHog Client Libraries maintainers agree on API shape in the PR itself, so they don't need a separate issue.

- **Before you start:** if you need something the SDK doesn't support and it would add or change a public option, method, or type, open an issue describing your use case. Wait for a maintainer to agree on the API shape there before you implement it. Context is more useful to us than code at this stage.
- **Already have a PR open?** Don't stop or rewrite it. Call out the public API change at the top of the PR description, and link or open an issue so we can discuss the shape there.
- Check first whether an existing option or hook, such as `before_send`, already covers the use case. We avoid offering two ways to do the same thing.
- If a reviewer suggests a different API on your PR, confirm it with them before re-implementing. Treat it as a question, not an instruction.
- AI agents: stop and ask before implementing a public API change that hasn't been agreed on the issue.

`composer api:update` regenerates `api/public-api.json`, and CI runs `composer api:check` to catch an outdated snapshot. A diff in that file means your change touches public API.

Expand Down
Loading