Skip to content
Open
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
49 changes: 49 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
version: 2

# Grouped: a week of transitive bumps arrives as one PR, not one per package.
updates:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

recommend a cooldown block to allow some time for releases to be vetted in the ecosystem for exploits/bugs before opening a PR

    cooldown:
      default-days: 7       # Fallback delay for all updates if not specified below
      semver-patch-days: 5   # Wait 5 days for patches (gives time to catch quick hotfix exploits)
      semver-minor-days: 14  # Wait 2 weeks for minor feature releases
      semver-major-days: 30  # Wait 30 days for major version changes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh good idea!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Claude working with Aaron:

Added in 66bd9cb, with your values.

GitHub Actions supports default-days only, so that ecosystem gets 7 days and no semver keys. Composer gets all four.

Cooldown covers version updates only. A security advisory still opens a pull request at once.

- package-ecosystem: composer
directory: "/"
schedule:
interval: weekly
day: monday
open-pull-requests-limit: 5
# This is a library. Widen the declared range, and keep the old versions
# that consumers still resolve.
versioning-strategy: widen

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Claude working with Aaron:

This file exists to make keeping composer.lock cheap.

The cost of a lock is churn. Grouping turns a week of transitive bumps into one pull request. cooldown lets a release age first, and it does not delay security updates.

versioning-strategy: widen protects the union range above. This is a library, so Dependabot must keep the versions that consumers still resolve.

# Let the ecosystem vet a release before we take it. Security updates ignore
# cooldown, so an advisory fix still arrives at once.
cooldown:
default-days: 7
semver-patch-days: 5
semver-minor-days: 14
semver-major-days: 30
groups:
# One PR for all open advisories, prod and dev.
composer-security:
applies-to: security-updates
patterns: ["*"]
# require-dev bumps reach nobody who installs the SDK.
composer-development:
applies-to: version-updates
dependency-type: development
patterns: ["*"]
# Majors stay ungrouped. They need a composer.json change and a release note.
composer-production:
applies-to: version-updates
dependency-type: production
patterns: ["*"]
update-types: ["minor", "patch"]

- package-ecosystem: github-actions
directory: "/"
schedule:
interval: weekly
day: monday
# The GitHub Actions ecosystem supports default-days only.
cooldown:
default-days: 7
groups:
github-actions:
applies-to: version-updates
patterns: ["*"]
75 changes: 64 additions & 11 deletions .github/workflows/run-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ on:

pull_request:

# Catches upstream releases that break our declared ranges.
schedule:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Claude working with Aaron:

A weekly run catches an upstream release that breaks a declared range.

The range is now wide, so a new Symfony release can break us without any commit here. The schedule finds it before a consumer does.

- cron: '17 6 * * 1'

workflow_dispatch:

workflow_call:
Expand All @@ -28,36 +32,85 @@ permissions:
contents: read

jobs:
build-and-test:
test-matrix:

runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
include:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Claude working with Aaron:

Six legs, one per reachable branch of the symfony/cache range.

config.platform.php pins the solver to PHP 8.1, so before this PR every resolution returned 6.4.x. The ^7.0 and ^8.0 branches were never tested. The highest legs unset that pin, so the runner PHP picks the branch: 8.1 takes 6.4, 8.3 takes 7.4, 8.4 takes 8.1.

fail-fast: false keeps every leg reporting, so one break does not hide another.

# The locked legs install what composer.lock pins. The lowest and
# highest legs resolve, and each Symfony major raises its PHP floor,
# so the runner PHP picks the branch of the symfony/cache constraint:
# 8.1 takes ^6.4, 8.3 takes ^7.0, 8.4 takes ^8.0.
- { php: '8.1', deps: locked }
- { php: '8.3', deps: locked }
- { php: '8.1', deps: lowest }
- { php: '8.1', deps: highest }
- { php: '8.3', deps: highest }
- { php: '8.4', deps: highest }
Comment thread
aarsilv marked this conversation as resolved.

name: PHP ${{ matrix.php }}, ${{ matrix.deps }}

steps:
- name: Display Testing Details
run: |
echo "Running SDK Test using"
Comment thread
aarsilv marked this conversation as resolved.
echo "Test Data: sdk-test-data@${TEST_DATA_BRANCH_NAME}"
echo "SDK Branch: php-sdk@${SDK_BRANCH_NAME}"
echo "PHP: ${{ matrix.php }}, dependencies: ${{ matrix.deps }}"

- uses: actions/checkout@v5
with:
repository: ${{ github.event.pull_request.head.repo.full_name || 'Eppo-exp/php-sdk' }}
# A called workflow sees the caller's github context, so github.repository
# is the caller's repo. Name this repo to stay correct on both paths.
repository: ${{ github.repository == 'Eppo-exp/php-sdk' && github.event.pull_request.head.repo.full_name || 'Eppo-exp/php-sdk' }}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Context from Claude working with Aaron: This expression looks redundant, so here is why it is not. A workflow called with workflow_call reads the caller's github context, so github.repository inside this file is Eppo-exp/sdk-test-data when test-sdks.yml calls it. Naming this repo explicitly keeps all four paths correct: a push here, a fork PR here, a cross-repo dispatch, and a cross-repo caller PR. The last one is broken on main today, and this repo's own CI cannot see it, because the bug only appears when someone else is the caller.

ref: ${{ env.SDK_BRANCH_NAME }}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
# No step needs git auth. make test clones test data from a public repo.
Comment thread
aarsilv marked this conversation as resolved.
persist-credentials: false

- name: Set up PHP
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php }}
extensions: json, pcntl, posix, sockets
coverage: none

# Must run before any resolution, or --strict cannot check lock sync.
- name: Validate composer.json and composer.lock
run: composer validate --strict

- name: Cache Composer packages
id: composer-cache
uses: actions/cache@v4
with:
path: vendor
key: ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }}
restore-keys: |
${{ runner.os }}-php-
# config.platform.php guards composer.lock. It must not cap a throwaway
# resolution, or symfony/cache ^8 stays unreachable.
- name: Target the runner's PHP for highest-version resolution
if: matrix.deps == 'highest'
run: composer config --unset platform.php

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Claude working with Aaron:

This step is what makes the highest legs meaningful.

config.platform.php guards the committed lock. If it also capped a throwaway resolution, symfony/cache ^8 would stay unreachable and the leg would pass without testing anything new.

Nothing is committed back. The runner rewrites its own copy of the lock.


- name: Install dependencies
run: composer install --prefer-dist --no-progress
uses: ramsey/composer-install@4.0.0
with:
dependency-versions: ${{ matrix.deps }}
composer-options: --prefer-dist

- name: Show resolved direct dependencies
run: composer show --direct

# --no-dev: a dev-only advisory reaches nobody who installs the SDK.
- name: Audit production dependencies

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Claude working with Aaron:

The audit runs on the locked legs, which is what consumers install.

--no-dev is deliberate. A dev-only advisory reaches nobody who installs the SDK. All six current advisories are dev-only, so this gate is green.

Known gap: the lock pins the 6.4 line, so an advisory against 7.x or 8.x is not covered. That is a follow-up decision about CI noise.

if: matrix.deps == 'locked'
run: composer audit --no-dev

- name: Run tests
run: make test branchName=${{env.TEST_DATA_BRANCH_NAME}}

# One stable check for every leg. Branch protection keeps the name
# build-and-test, whatever the matrix holds.
build-and-test:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Claude working with Aaron:

This job keeps branch protection working, whatever the matrix holds.

A matrix renames a job into one check per leg. Any required check named build-and-test would stop reporting. This job restores that name and fails if any leg fails.

Verified with a deliberate failure: the leg failed, and this job failed with it.

if: always()
needs: test-matrix
runs-on: ubuntu-latest
steps:
- name: Check the matrix result
run: |
echo "Matrix result: ${{ needs.test-matrix.result }}"
[ "${{ needs.test-matrix.result }}" = "success" ]
1 change: 0 additions & 1 deletion .github/workflows/test-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ name: Package Integration Testing
on:
push:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Claude working with Aaron:

This trigger started a package test for every new branch.

The workflow declared on: create with a tags filter. GitHub ignores filters on create, so every Dependabot branch started a run, and each one failed.

A tag push honours the filter. Note the separate problem: the packaging test also fails on pushes to main, inside the shared sdk-test-data action. That needs its own ticket.

branches: [ main ]
create:
tags:
- v*

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,4 @@ test-data:

.PHONY: test
test: test-data
./vendor/phpunit/phpunit/phpunit tests
./vendor/bin/phpunit tests
6 changes: 3 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@
"ext-json": "*",
"psr/simple-cache": "3.*",
"psr/log": "^2.0|^3.0",
"shrikeh/teapot": "^2.3",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is the reason/motivation to remove teapot?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe it wasn't being used but can double check!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Claude working with Aaron:

Correction to my reply above: teapot was in use. It supplied HTTP status constants at three import sites on main:

  • src/API/APIRequestWrapper.php:10RFC7231, used in isHttpErrorRecoverable
  • tests/API/APIRequestWrapperTest.php:17-18RFC7231 and RFC7235, used in ten places

PR #64 inlined the literals. That is what created the magic numbers you flagged in the other thread.

The removal still holds. The package was a production dependency that named integers and did nothing else. 66bd9cb replaces those integers with private constants on APIRequestWrapper, named per RFC 7231 and RFC 7235.

The tests keep literal status codes on purpose. A test that reads the same constant as the code cannot detect a wrong constant value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Context from Claude working with Aaron: One update to the reply above. The tests now name their statuses from the RFC again, via teapot/status-code ^2.1 in require-dev (4b6a150).

shrikeh/teapot stays out of require, which is @pkruithof's change. teapot/status-code is its constants-only sibling: no PSR-7 decorator, and no psr/http-message ^1.0 cap, which the parent package would have imposed on the lock. Production keeps the private constants you asked for.

"composer/semver": "^3.4",
"php-http/discovery": "^1.17",
"webclient/ext-redirect": "^2.0",
"symfony/cache": "^6.4|^7.0"
"symfony/cache": "^6.4|^7.0|^8.0"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Claude working with Aaron:

This line is the point of the PR, and it is @pkruithof's commit, unchanged.

The branch keeps his widening and his shrikeh/teapot removal. It restores composer.lock, which he later agreed to keep.

GitHub builds the dependency graph from the lock, and that graph feeds our security alerts. FFESUPPORT-534, -734 and -887 all came from it.

},
"require-dev": {
"phpunit/phpunit": "^9.6",
Expand All @@ -39,7 +38,8 @@
"ext-pcntl": "*",
"psr-mock/http": "^1.0",
"squizlabs/php_codesniffer": "^3.10",
"ext-sockets": "*"
"ext-sockets": "*",
"teapot/status-code": "^2.1"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Context from Claude working with Aaron: This is the constants-only sibling of shrikeh/teapot, and it is dev-only, so require keeps exactly what #64 left and consumers install nothing new. Two reasons for this package and not shrikeh/teapot: that one also ships a PSR-7 response decorator we never used, and it caps psr/http-message at ^1.0, which would pin the lock to 1.x. teapot/status-code declares only a PHP constraint, and it covers 8.1 through 8.5, so every matrix leg resolves it.

},
"config": {
"allow-plugins": {
Expand Down
161 changes: 56 additions & 105 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading