fix(FFESUPPORT-969): re-lock php relay for PHP 8.3; honor sdk_ref, fail fast, pin checkout in package tests

[aarsilv]

🤖 Generated from Claude

Jira: FFESUPPORT-969. PHP package tests fail since 2026-06-22. This PR fixes them, plus 3 harness bugs found on the way.

#	Change	Why
1	php relay: lock for PHP 8.3 (config.platform.php 8.3.0, require.php ^8.3, Dockerfile php:8.3). symfony/options-resolver v8.1.0 → v7.4.8. No other package changes.	Lock needed PHP ≥ 8.4.1. Runner has 8.3.6. composer install failed.
2	php relay build-and-run.sh: set -e. Check out the SDK in a private mktemp -d dir, removed after the copy and on exit.	Script continued after errors. A failed clone silently tested the locked SDK (v4.2.1). A fixed relative tmp/ depended on the caller's cwd.
3	test-sdk.sh: honor sdk_ref (argument → env → main). php and node relays fetch the ref: branch, tag, full SHA, or <N>/merge.	test-sdk.sh:59 reset SDK_REF to main. Relays that read it always got main.
4	test-sdk.sh: stop when the relay launch script exits non-zero. Print the last 50 lines of sdk.log.	A dead relay cost 5 min. The error was only in the artifact.
5	Action: check out sdk-test-data at the ref the consumer pinned (@ref in uses:). No ref → today's default.	Consumers always got main harness files.

CI (manual dispatch; no workflow runs on PRs for these paths): Test Packaged SDKs at 5140fd9, the head after the rebase on main (#163 merged) (run): all 6 jobs pass, php 264/264. Every job pulled the testing-api and runner images that #163's release workflows pushed. Before the rebase, at ba55e6e (run): all 6 jobs pass, php 264/264. Probe run: pinned checkout works, v4.2.1 and 70/merge pass, a bad ref fails in 5 s.

After merge (both consumers pin @main, so they get all changes at the next run):

• php-sdk: package tests get the PHP 8.3 fix. v* tag runs test the tag, not main.
• node-server-sdk: release runs test the tag (Node 20, 22, 24). A failure blocks the npm publish.
• Tag runs can show failures that main runs hid.

Details: php relay (changes 1–2)

• Root cause. composer install refused the lock (see the sdk.log artifact). No set -e, so the script started php -S with no vendor/. Each request failed, and the health check timed out after 5 min. Examples: main run 30020967684, branch run 36277178730.
• Targeted update. composer update symfony/options-resolver only. A full update changes 15 more packages and removes 2 (incl. psr/http-message 1.1 → 2.0, symfony/cache 6.4 → 7.4). The relay replaces only vendor/eppo/php-sdk/, so the rest stays as locked. Lock also gets platform.php ^8.3, a new content-hash, and platform-overrides.
• Why 8.3.0. CI runs 8.3.6. Composer reads 8.3 as 8.3.0. php-sdk uses the same X.Y.Z form (8.1.0). Today 8.3.0 and 8.3.6 resolve the same. The solver never picks a package whose PHP floor is above 8.3.0. Not 8.1.0: PHP 8.1 is end of life, and CI does not run it.
• Why require.php ^8.3. The lock needs PHP ≥ 8.2, so ^8.1 was wrong. ^8.3 matches the platform value. platform_check.php now requires ≥ 8.3.0.
• Why set -e. A failed composer install, SDK checkout, or cp into vendor/ now stops the script at the real error. Before, the clone's exit 1 ran in a subshell, cp copied nothing, and the relay tested v4.2.1. php src/eppo_poller.php & runs in the background (not affected). php -S is last.
• Why mktemp -d. main used a fixed relative tmp/ and deleted it. That path depends on the caller's cwd (CI: the relay dir via pushd; Docker: /relayapp), so a manual run elsewhere could delete an unrelated tmp/. A fresh private dir per run also keeps stale files from a failed run out of vendor/. An EXIT trap removes it when the fetch fails. Nothing else in the relay uses tmp/.
• Why the Dockerfile. With the override, composer install checks against 8.3.0, not the real PHP. On PHP 8.1 the install passes, then each request fails in platform_check.php. CI does not use this Dockerfile. docker-run.sh and the README do.

Details: sdk_ref (change 3)

• The action passes sdk_ref in env SDK_REF and calls test-sdk.sh with 2 arguments, so ${3:-main} always gave main. Order now: sdkRef argument, then SDK_REF from .env or the shell (.env wins, as it does for the other env defaults such as EPPO_API_HOST), then main. Before, .env also beat the argument.
• php and node use git init + git fetch --depth 1 + git checkout FETCH_HEAD. git clone -b takes only a branch or tag. <N>/merge maps to refs/pull/<N>/merge. Each relay logs the commit it checks out.
• Other relays: python, ruby, android still git clone -b. dotnet clones only when SDK_REF is not empty (always, as before). java and go do not read SDK_REF. node still prefers SDK_VERSION; no consumer sets it. In this repo the workflow passes no sdk_ref, so every relay still gets main.

Details: fail fast (change 4)

• wait_for_url gets the launch script PID, on all 3 launch paths (build-and-run.sh, docker-run.sh, build-and-run-<platform>.sh). Non-zero exit: stop at the next failed poll (5 s interval), message "SDK Relay launch script exited with status N", job exits 1.
• Exit 0: polling continues, because such a script can leave a server running (docker run -d). So it cannot trip this check.
• Each launch script CI runs ends with a foreground server. go's docker-run.sh has no -d. php's docker-run.sh has -d, but build-and-run.sh runs first. android (not in CI) exits after adb shell am start: 0 on success, so polling continues. The check covers startup only.

Details: pinned checkout (change 5)

• A new step reads github.action_repository and github.action_ref through step env, the form the contexts docs name for composite actions. Direct reads in run are empty (actions/runner#2473).
• Action from Eppo-exp/sdk-test-data → checkout that ref. Else (local ./ action, empty, other repo) → empty ref. In actions/checkout@v3, empty = no ref: the triggering ref here, else the default branch. Worst case = today's behavior. The step logs both values and its choice.

Evidence

CI

• 37080733417 at 5140fd9 (head: the same 9 commits rebased on main 33da68c, which includes fix(FFESUPPORT-999): remediate September 2026 vulnerabilities #163; git range-diff shows each commit unchanged): all 6 jobs pass. php 264/264, node 264/264, java 256/258 (2 skipped), go/ruby/python 229/239 (10 skipped). Each job pulled runner sha256:a57af999… and testing-api sha256:c928cf0b…, the digests that fix(FFESUPPORT-999): remediate September 2026 vulnerabilities #163's release workflows pushed.
• 36336301942 at ba55e6e (head before the rebase, .env precedence fix): all 6 jobs pass, php 264/264.
• 36335586518 at 20eb741 (adds mktemp): all 6 jobs pass, php 264/264.
• 36331671515 at 9863d37: php 264/264 (PHP 8.3.6, options-resolver v7.4.8), node 264/264, java 256/258 (2 skipped), go/ruby/python 229/239 (10 skipped). Resolve step logged empty values (local action), so checkout used its default.
• 36284484238 at c64282b (changes 1–2 only): all pass, php 264/264.
• Probe 36331701740 at e5635c1 (temporary commit, since removed; 3 php jobs):
  • action @9863d37… with sdk_ref: v4.2.1: checked out sdk-test-data 9863d37; relay checked out php-sdk 9e25253 (v4.2.1); 264/264.
  • sdk_ref: 70/merge: relay checked out 2deacf7; 264/264.
  • sdk_ref: no-such-ref-969: failed 5 s after relay start, with "couldn't find remote ref" and "launch script exited with status 1" in the step log (expected).

After merge, a push to main runs the same workflow.

Local (docker; real test-sdk.sh, stub docker where noted). CI pulls the published latest testing-api and runner images; the local full tests built them from this repo. CI clones the SDK at run time.

• Change 1: origin/main lock fails on PHP 8.3.6 and 8.3.35. New lock installs on 8.3.6, 8.3.35, 8.4.26, 8.5.11. Full package test with images built from this repo: 264/264 on 8.3.6. composer audit --locked and OSV (42 packages): no advisories. On 8.5.11, a full composer update --dry-run keeps v7.4.8; without config.platform it picks v8.1.0.
• Change 3: precedence correct on the real script lines: arg beats .env and shell; .env beats shell; shell alone used; nothing → main. php relay checks out the ls-remote commit for main, a slashed branch, tag v4.3.0, a full SHA, 70/merge, refs/pull/68/merge. Bad ref exits 1 in 3 s. Full tests: php v4.2.1 264/264, php 70/merge 264/264, node v4.0.1 264/264, node 127/merge 264/264.
• Change 4 (stub docker): healthy php relay passes. main lock: stops in ~5 s, status 2, options-resolver error shown. Bad SDK_REF: status 1. Missing command: 127. Platform script exit 5: status 5. Background server + exit 0: passes. Exit 0 with no server: keeps polling. On macOS bash 3.2.57, statuses 3, 127, and 5, the background-server case, and a platform script with a foreground server give the same results.
• mktemp (relay Dockerfile image): v4.2.1 relay answers HTTP 200, checks out 9e25253; an unrelated tmp/sentinel in the cwd survives; no temp dir left. Bad ref: exit 1 in < 1 s, temp dir removed, sentinel intact. mktemp -d also works on macOS.
• Change 5: resolve script gives main for (Eppo-exp/sdk-test-data, main); empty for (actions/checkout, v3), (empty, empty), (Eppo-exp/sdk-test-data, empty).

Not validated · follow-ups

Not validated

• Consumer workflows (they use the action from main). The probe used the same code path at a pinned SHA.
• A pull_request run in a consumer: neither has one. <N>/merge tested locally and in the probe.
• macOS and Windows runners (workflows use Linux only). The resolve step avoids bash 4 syntax.

Follow-ups

• python, ruby, android, dotnet still git clone -b (branch or tag only). python's || ( echo ...; exit 1 ) does not stop its script.
• action.yml says sdk_version overrides sdk_ref. Of the CI relays, only node reads SDK_VERSION.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

Align the root PHP requirement with the PHP 8.2+ dependency lock or retain a PHP 8.1-compatible lock.

Review effort: Lite
Findings: 1

Open (1)

• Composer override breaks PHP 8.1 compatibility · New

What changed in this PR

Updates the PHP SDK relay for PHP 8.3 compatibility and improves startup failure handling.

Changes:

• Pins Composer resolution to PHP 8.3 and re-locks compatible dependencies.
• Updates the Docker image to PHP 8.3.
• Enables fail-fast setup and cleans stale temporary checkouts.

File	Summary
package-testing/​php-sdk-relay/​Dockerfile	Uses PHP 8.3.
package-testing/​php-sdk-relay/​composer.lock	Locks PHP 8.3-compatible dependencies.
package-testing/​php-sdk-relay/​composer.json	Adds the PHP platform override; the PHP 8.1 requirement conflicts with the resulting lock.
package-testing/​php-sdk-relay/​build-and-run.sh	Adds fail-fast behavior and temporary-directory cleanup.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟢 Approval recommended

The reviewed changes are consistent, tested, and have no unresolved blocking issues.

Review effort: Lite
Findings: None

Resolved since last review (1)

• Composer override breaks PHP 8.1 compatibility

[aarsilv]

🤖 Response from codex: Fact-check of this PR description, 3 rounds, against the diff, the repo files, and the author's executed evidence (docker runs, CI API results).

• Round 1 (20 claims): FIX FIRST. Fixed: "16 other packages" (15 besides options-resolver); "same lock" for 8.3.0 vs 8.3.6 (same package versions; lock metadata differs); "the lock runs on every 8.3 release" (narrowed to solver behavior); "every run tests SDK main" (a .env can override); the docker run -d follow-up example (go runs in the foreground); "fails on every branch"; "breaks all consumers" (node uses another relay); missing CI results, SDK commit, and image provenance.
• Round 2: all round-1 objections FIXED except one new defect: the lock's platform entry was called "new" (it existed; only its php value changed). SDK commit added.
• Round 3: both FIXED. INTRODUCED: NONE.

VERDICT: POST AS IS - Both edits match the supplied evidence.

[Copilot]

Copilot review overview

🟢 Approval recommended

No unresolved review issues were identified.

Review effort: Lite
Findings: None

[aarsilv]

🤖 Response from codex: Fact-check of the updated PR description (four changes), against the diff, the repo files, and the author's executed evidence (docker runs; CI run and probe-run logs from the GitHub API). One run per section, iterated until no factual defect remained.

Section	Rounds	Corrected before the final verdict
1. Re-lock php relay	1	none
2. Honor sdk_ref	2	"every run tested SDK main" (go and java never read SDK_REF)
3. Fail fast	4	"stops at once" → next failed poll; foreground claim scoped to CI launchers (php docker-run.sh uses -d, android exits); android status 0 only on success; sleep length stated as a sleep, not a bound; "at least 5 minutes"; "a healthy relay cannot trip" → "a launch script that exits 0 cannot trip"
4. Pinned checkout	2	"another action's values" → "values that name another repository"
CI and local results	2	macOS bash 3.2 claim narrowed to the scenarios run; launcher status vs harness exit 1
Consumer impact, not validated, follow-ups	2	fail-fast qualified to startup failures; failure history bounded to June 22 – August 31; sdk_version claim limited to CI relays; dotnet added to follow-ups

VERDICT: POST AS IS (every section, on the posted text).

[aarsilv]

🤖 Response from codex:

Review of 20eb741 (private mktemp -d checkout in the php relay) and the description lines for it.

• Code: no blocking issues found. Paths are quoted, and cleanup runs after the copy and through the EXIT trap on a failed fetch.
• Claims: 18 CONFIRMED. 2 UNVERIFIABLE from the evidence given: the sentinel after the bad-ref run, and "manual dispatch" for run 36335586518. Claude checked both against the executed results. The test log shows sentinel still there: keep after the bad-ref run. The run API shows event=workflow_dispatch. No change to the description.

[Copilot]

Copilot review overview

🔵 Needs a closer look

An unresolved moderate issue causes .env to override an explicitly supplied SDK ref, violating the documented precedence.

Review effort: Lite
Findings: None

[aarsilv]

🤖 Response from Claude:

Re the Copilot review overview on 20eb741: ".env overrides an explicitly supplied SDK ref, violating the documented precedence." Agreed.

• test-sdk.sh resolved SDK_REF and then sourced .env. So a SDK_REF line in .env beat the sdkRef argument. The script header and the README say the argument wins. main had the same order, but this PR documents the precedence, so the code must match it.
• Commit ba55e6e resolves SDK_REF after .env, next to the other env defaults. Order: argument, then SDK_REF from .env or the shell (.env wins, as it does for the other env defaults such as EPPO_API_HOST), then main.
• Checked on the real script lines: argument v1 with .env v3 and shell v2 gives v1. No argument gives v3. Shell only gives v2. Nothing gives main.
• CI does not use a .env file (.env is gitignored; only .env.EXAMPLE is tracked, and it has no SDK_REF).

[aarsilv]

🤖 Response from codex:

Review of ba55e6e (sdkRef argument wins over .env) and the description lines for it.

• Code: no blocking issues found.
• Claims: one WRONG, now fixed: ".env wins, as for every other variable" overstated it, because EPPO_BASE_URL is always recomputed from EPPO_API_HOST and EPPO_API_PORT. The text now says "as it does for the other env defaults such as EPPO_API_HOST". The other claims are CONFIRMED, or were already verified by earlier executed runs. The run API shows event=workflow_dispatch for run 36336301942.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Relay failure detection should correctly handle exited child processes before approval.

Review effort: Lite
Findings: None

[aarsilv]

🤖 Response from Claude:

Re the Copilot review overview on ba55e6e: "Relay failure detection should correctly handle exited child processes." It has no line finding. I checked it, and I will not change the code.

wait_for_url checks kill -0 "$pid" and then calls wait "$pid" for the exit status. The launcher is a direct child (SDK_RELAY_PID=$! in the same shell).

• Exited and reaped: handled. Bash keeps the status of a reaped background child, so wait still returns it. codex ran this on bash 3.2.57: kill -0 gave 1, and wait gave 3.
• Exited, still a zombie at the check: kill -0 succeeds, so detection moves to the next poll, 5 seconds later. Nothing is missed.
• PID reuse after exit: in theory, kill -0 can match another process. Then the harness polls until the timeout, which is the behavior before this PR. It is not a new failure.
• Executed: with the real test-sdk.sh and a stub docker, launch scripts that exit 3, 127, and 5 fail within about 5 seconds with the correct status, on Linux bash and macOS bash 3.2.57. A script that starts a background server and exits 0 keeps polling and passes.

codex adjudicated this separately and returned REPLY (no change).

[Copilot]

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: None