PMK-2061: current PHP versions, and a pipeline that reports the truth (v8.0.0)

[ewood-ac]

Jira: PMK-2061

Stacked on #167 (PMK-2059 / v7.0.1) — review that first; this branch contains it.

The suite was red on main: 87 tests, 20 errors, 2 failures. Almost none of it was the SDK. This gets to 92 tests, 0 errors, 0 failures without papering over anything — the account-state cases now skip with a message naming exactly what's missing, so a configuration problem is distinguishable from a defect.

	main	this branch
Errors	20	0
Failures	2	0
PHPStan	green	green

PHP versions

Collapsed the constraint to ^8.2. The previous ~8.1 || ~8.2 || ~8.3 || ~8.4 already resolved to >=8.1 <9.0, so 8.5 was always permitted — the old CHANGELOG line about "adding 8.5 to the supported range" was misleading. The only real change is dropping 8.1 (EOL 2025-12-31).

• Added php85 to the matrix; moved static-analysis off the now-dropped 8.1 image.
• Unchained the serial requires — one slow version was hiding the other three and quadrupling wall-clock.
• README.md said "PHP 8.1, 8.2, 8.3, or 8.4" — wrong in both directions.

Sender signatures — the root cause

The tests built their address with str_replace('[TOKEN]', …, $prototype). That's only unique if the configured prototype contains the placeholder. testing_keys.json.example documents anything+[token]@wildbit.com, but the value in use is a plain address with no placeholder — so the replace was a no-op and every run tried to create the same signature:

This signature already exists.
This Sender Signature has already been confirmed.

The case never matched either: the example is lowercase, the tests replaced uppercase. uniqueSenderAddress() handles both shapes, and those tests now pass rather than erroring.

The 10 'From' address is not a Sender Signature errors are genuinely account state — nothing in code can confirm a signature. They now skip once, naming the address and the fix, instead of failing ten times in a way that reads like an SDK fault. Same for the statistics token.

Making the rest honest rather than green

• Empty-fixture assertions (search returning 0, Opens[0] on an empty list) skip with the reason instead of failing on an array offset.
• Stream archiving is refused by the API for a stream created moments earlier. archiveOrSkip() retries, then skips with the API's own message — and rethrows anything that isn't that specific condition, so a real archive regression still fails.

I've deliberately kept these as visible skips with reasons rather than weakened assertions. A skip that says why is recoverable; assertGreaterThanOrEqual(1) is silently permanent.

Two silent data bugs, both in released versions

• getDeliveryStatistics() has reported Count = 0 for every bounce category, in every version. PostmarkBounceSummary read $values['FirstOpen'] instead of $values['Count'] — a copy-paste from PostmarkOpen. Anything calibrated against the broken zero will start seeing real numbers.
• PostmarkBounce assigned its constructor fallbacks to the wrong properties — Type (string) got 0, TypeCode (int) got '' — so a response missing either field threw a TypeError.

Breaking changes (hence v8.0.0)

1. PHP 8.1 dropped. Composer won't offer v8 to 8.1 projects.
2. PostmarkAttachment::fromRawData() / ::fromBase64EncodedData() / ::fromFile() declare string params and a PostmarkAttachment return. Subclasses overriding them must add the return type or PHP fatals at class-load.
3. PostmarkAttachment::fromFile() throws RuntimeException when the file can't be read. It previously discarded file_get_contents()'s false, and base64_encode(false) is "" — so an unreadable path shipped an empty attachment with the message.

Also

Guarded the remaining 10 list models against a missing collection key (only 5 had been done). Removed the returns after markTestSkipped() — it's @return never, and they were the sole reason PHPStan was red on the old #164 branch.

Supersedes #164

Same ticket family, but #164 predates the v7.0.1 split and its two nullable fixes now ship in #167. This branch contains everything #164 had that's still relevant. Close #164 in favour of this once you're happy.

[ac-bgelfius]

Final review — postmark-php #168 (v8.0.0) @ d4a6a698

Recommendation: request-changes — one verified hole in the new credential guard reproduces
the exact fatal this release exists to eliminate, in the test file the PR touches most. Everything
else is documentation placement and CI hardening. The code changes are right.

Base is #167's branch (pmk-2059-nullable-getter-fatals), not main. #167 is reviewed separately;
nothing below is filed against it.

---

Start here: this is good work, and several parts are better than the PR says

• Both silent data bugs are real and correctly fixed. PostmarkBounceSummary.php:15 read
  $values['FirstOpen'] into public int $Count, and bounce-summary items carry no FirstOpen
  field at all — so getDeliveryStatistics() has reported zero for every bounce category in every
  released version. Independently verified by two seats. The CHANGELOG's warning that dashboards
  calibrated on that zero will start seeing real numbers is exactly the right call-out.
• The undeclared-break sweep came back clean. The architect checked mechanically across both
  PRs — every removed/changed public/function/class line in src/ — and found no removed
  public method, no visibility change, no undeclared signature change. The only breaks are the
  three PostmarkAttachment factories, all declared. For a major on a published SDK, that is the
  single most useful thing a consumer can be told, and it holds up.
• Guzzle 7 and 8 are both genuinely supported. Verified by resolving each separately (7.15.3
  and 8.0.2), running PHPStan clean on both, and loading all 85 SDK classes on both. The matrix is
  buying real signal, not ceremony.
• The composer constraint arithmetic is right. ~8.1 || ~8.2 || ~8.3 || ~8.4 really did resolve
  to >=8.1 <9.0, so 8.5 was always permitted. The author is correct that the earlier "added 8.5"
  CHANGELOG line was misleading, and correcting your own prior imprecision in a release note is the
  right instinct.
• archiveOrSkip() genuinely rethrows anything that isn't the cooldown — verified structurally:
  it catches only PostmarkException, and rethrows immediately when the message doesn't match. The
  test philosophy behind it is the right call. Visible skips with reasons beat weakened assertions,
  and the diagnosis was exact — the 79 identical credential TypeErrors were reproduced.
• The CI pipeline rework is a correctness fix, not a wall-clock optimisation — and neither the PR
  body nor the CHANGELOG says so. Because php81 is first in the requires chain and has been red
  since 2025-10-24, php82/php83/php84 have not executed at all for roughly ten months. Not
  failed — never started, no commit status posted. Confirmed two ways: the cost seat from CircleCI's
  requires semantics, and the CircleCI seat directly against real commit statuses (ad4b80e posts
  only php81; the green commit before it posts all four). Unchaining ends a ten-month coverage
  blackout. That is a far stronger justification than "chaining quadrupled wall-clock," and it is
  worth putting in the PR body — it is the argument for never chaining a version matrix again.

---

Blockers

None. The request-changes rests on the should-fix items below — specifically the first one.

Should-fix

1. The credential guard has a hole, in the file this PR touches most — this is what gates the PR.
tests/PostmarkClientBounceTest.php:29, plus PostmarkClientEmailTest.php:26,
PostmarkClientEmailsAsStringOrArrayTest.php:16, PostmarkClientStatisticsTest.php:16,
PostmarkClientTemplatesTest.php:54 — all five override setUp() without calling
parent::setUp(), so the base guard never runs for them.

The bounce class is the sharp case. Its setUp() calls only requireConfirmedSenderSignature(),
which checks WRITE_ACCOUNT_TOKEN and WRITE_TEST_SENDER_EMAIL_ADDRESS. But
testClientCanGetBounce() (:37) and testClientCanGetBounceDump() construct
new PostmarkClient($tk->READ_SELENIUM_TEST_SERVER_TOKEN, …) — and nothing anywhere checks that
token. An environment with write credentials but no Selenium token sails past every guard and
gets back Argument #1 ($serverToken) must be of type string, null given: the exact fatal this PR
exists to eliminate.

One correction that makes the fix bigger than "chain the parent": chaining parent::setUp()
alone does not close it. The base guard is hasAnyCredentials() (tests/TestingKeys.php:56-72),
which returns true if any one of six tokens is set. A partially-configured environment passes it.
The per-key requireKeys() is the part that actually guards. Same gap on WRITE_TEST_SERVER_TOKEN,
which PostmarkClientBounceTest:74, EmailTest:35, EmailsAsStringOrArrayTest:24 and
TemplatesTest:28 all build clients from and no guard checks. PostmarkClientStatisticsTest is the
one that got it right — it guards the exact token it uses.

Fix: chain parent::setUp() in all five, and add requireKeys() for the token each class actually
constructs a client with — READ_SELENIUM_TEST_SERVER_TOKEN on the bounce class,
WRITE_TEST_SERVER_TOKEN on the other three.

2. CHANGELOG.md:57-79 — the Guzzle 8 exception reclassification is filed under ### Added.
This is the largest blast radius in v8.0.0. A catch (ConnectException $e) around a send silently
stops matching a timeout, and a swallowed timeout is worse than a fatal because nothing tells you.
The entry's content is excellent; it is sitting where nobody scanning for BREAKING will read it.
Move it under ### Changed with the same BREAKING marker the other two breaks get. Mis-filing
the compatibility disclosure is not a heading nit when the disclosure is the point of the release.

3. composer.json:13 — the Guzzle floor rises ^7.8 → ^7.15.2, described inside the "Added"
paragraph. The security rationale is sound and stated, but this is a requirement change that can
block composer update for a consumer whose other dependencies pin an older Guzzle 7.x. Put it
under ### Changed next to the PHP floor.

4. .circleci/config.yml:36-37 — integration-tests exits 0 having executed zero tests when
unconfigured (verified). That is the mirror image of the problem being fixed: the old pipeline
screamed when credentials went away, the new one goes quietly green. Skipping is right for a fork
PR — CircleCI withholds project env vars from fork builds by design, and the PR correctly relies on
that. On the trunk build the job should fail if it could not run. This isn't gold-plating: this
very repo just demonstrated a silent-skip failure mode running undetected for ten months. A
test -n "$WRITE_ACCOUNT_TOKEN" before composer test:integration, or --fail-on-skipped, closes it.

5. CHANGELOG.md:86-87 — the PostmarkBounce note is half wrong, and it undersells its own bug.
It says Type got 0 and TypeCode got '', "throwing TypeError on a response missing either
field." Only TypeCode threw. Type silently coerced 0 to the string "0", because no src/
file declares strict_types. Two seats found this independently. Correct it — silent corruption
is the stronger case for the fix, and a reader who checks will find the note doesn't match.

6. The PR body's "0 errors / 0 failures" table needs a caveat. It is the locally-verified full
suite; CircleCI's integration job at this head shows Tests: 87, Errors: 4, Skipped: 31 (build
704, pulled via CircleCI's public v1.1 API — no token needed, the repo is public). The 92-vs-87 test
count reconciles fine (full suite vs --testsuite integration). The 21→4 error reduction is real
and is your fix working. But as written the table reads as if CI agrees, and it visibly doesn't.
See the open question below on the 4th error.

7. src/Postmark/Models/PostmarkAttachment.php:41,44 — @file_get_contents() suppresses the
reason. The new RuntimeException can only say "Unable to read attachment file X" — not missing
vs unreadable vs failed stream wrapper. For an SDK that's the difference between a one-minute fix
and a support ticket. Fold in error_get_last()['message'], or drop the @.

8. Consider a short "Upgrading from v7" section at the top of the v8 entry. Two separate items —
the bounce-count fix and fromFile() now throwing — are both "your program's behaviour changes
though your code didn't," and they currently sit under different headings. A customer whose file-path
generation has a latent bug starts getting hard failures on upgrade; a customer with a bounce-rate
alert starts getting real numbers. Both are correctly documented; neither is prominent. Worth a
callout on the GitHub Release too, which is the artifact more people actually see.

Nits

• phpunit.xml.dist:15 — after the split, the unit suite is a single hardcoded <file>. So
  php82/php83/php84/php85 each run ~5 assertions over 3-4 model classes, and the full suite
  runs on 8.2 only. Before, all tests ran on four versions (red). The honesty gain is real and
  worth it; the per-version depth loss is also real and isn't mentioned in the PR body or the
  CHANGELOG. Two cheap improvements: <directory>tests/Unit</directory> so the next unit test isn't
  silently skipped, and a class-load smoke test over src/ — the php seat wrote one and it loaded
  all 85 classes clean on both Guzzle majors, which would make the config comment ("a red one means
  the code is broken on that version") actually true.
• CHANGELOG.md:39 — the v8.0.0 block sits below ## [v7.0.1]; the file is otherwise newest-first.
  Someone opening it to see the latest release can miss the entire breaking block.
• tests/PostmarkClientMessageStreamsTest.php:47 — stripos($lastMessage, 'unable to be archived')
  is broader than the cooldown it documents. PostmarkException::getPostmarkApiErrorCode() already
  carries the API's ErrorCode; matching on that makes the skip as narrow as the comment claims.
  [NEEDS VERIFICATION: which ErrorCode the cooldown returns — nobody called the API.]
• tests/PostmarkClientBounceTest.php:26-29 — the @depends testClientCanActivateBounce docblock
  rode onto setUp() in the move, where @depends means nothing, and testClientCanGetBounce()
  lost it. Some new lines carry trailing whitespace.
• PostmarkBounceList.php:14 and four other list models assign ?? [] to a local first; the other
  eight guard inline. Same fix, two shapes, one diff.
• php-cs-fixer now runs twice per push, for zero added coverage — it lives inside the parameterized
  static-analysis job template, which is invoked as both static-guzzle7 and static-guzzle8.
  PHPStan legitimately needs both (different type surface); the formatter doesn't.
• The unchaining is attributed purely to wall-clock. On the credit axis the last ten months ran the
  other way — the broken chain was suppressing ~75% of the matrix cost by never starting three
  jobs. One sentence completes the story. (Absolute cost either way is cents to a few dollars a
  year: one merge to main in the trailing 12 months.)
• The suite now carries 13 markTestSkipped() sites across 6 files. A green run with 13 skips looks
  identical to a green run with 0 unless someone opens the log. Not asking for it here — worth a
  guardrail before the norm scales.

Open questions

• The 4th integration error is not explained, and it might be yours.
  testClientCanRequestNewVerificationForSignature creates a brand-new uniquely-suffixed signature
  via this PR's own uniqueSenderAddress() and immediately gets back "This Sender Signature has
  already been confirmed." Three of the four errors are a confirmed-environmental stale
  READ_SELENIUM_OPEN_TRACKING_TOKEN. This one is explicitly [INFERRED] — the plausible story is
  that the domain behind WRITE_TEST_SENDER_SIGNATURE_PROTOTYPE auto-confirms new signatures, which
  is account state. But if it's a real interaction between the new dedup logic and an auto-confirming
  domain, that's a gap in the fix rather than CI drift, and unlike archiveOrSkip() this test has no
  skip path for it. One class confirmed environmental, one plausible but unconfirmed — please
  close the second one rather than filing the whole red under "account state."
• PostmarkAttachment.php:44 throws \RuntimeException rather than something under
  Postmark\Models. A consumer wrapping SDK calls in catch (PostmarkException $e) won't catch it.
  SPL is defensible for a filesystem failure, but v8 locks the choice in until v9 — worth being
  deliberate rather than incidental.
• Has anyone checked ^7.15.2 against a realistic consumer whose other dependencies pin an older
  Guzzle 7.x? The floor bump is either theoretical or a real composer update blocker, and nobody
  on the panel established which. [NEEDS VERIFICATION]
• Is there any CI-health check on this repo independent of PR activity? php81 broke on 2025-10-24
  and nothing surfaced it for ten months because nothing else was merged. #168 resolves it by
  deleting the job, which is defensible (8.1 is EOL) but means "what actually broke php81" was routed
  around rather than answered. For the record: the CircleCI seat pulled build 678's output — all 21
  errors and 3 failures were account-state/credential problems, no PHP-8.1 TypeError or syntax
  failure. So the answer is "nothing about 8.1," and that's worth knowing.
• Neither PR has a requested reviewer or a recorded review, and there's no CODEOWNERS. For a
  publicly-consumed SDK making a major-version breaking call, is there meant to be a designated
  second sign-off, and does that person know these are up?
• How does Postmark announce an SDK major beyond CHANGELOG.md? [NEEDS POSTMARK VERIFICATION —
  nothing in the checkout or the system map says.]

Out-of-scope observations

Not filed against this PR — the files are untouched and nothing in the diff makes them more or less
likely to fire. Listed so the sweep reads as closed rather than half-open.

• src/Postmark/Models/TemplateValidationResponse.php:197 — TextBody::$RenderedContent falls back
  to 0 where its two siblings in the same file (HtmlBody:92, Subject:252) use '', so an
  HTML-only template gets "0" where the truth is "". Unambiguously a typo, and it does produce
  a wrong value rather than coercing to a sensible one — worth your attention while you're here. It
  gates nothing: "this PR fixes two instances of a defect class and misses a third" is a completeness
  argument, not an aggravation argument.
• TemplateValidationResponse.php:93,198,253 — the bigger one in that file. All three body classes
  discard the false branch of a ternary, leaving public array $ValidationErrors uninitialized, so
  getValidationErrors() throws "must not be accessed before initialization" on the happy path
  of validateTemplate(). Verified against a realistic payload. Same shape as #167's bug, three files
  over. One-line fix each.
• PostmarkOpen.php:19, PostmarkServer.php:48, PostmarkServer.php:51 — three more fallback/type
  mismatches, all benign today because weak mode coerces them, all of which would fail loudly the
  moment strict_types is added.
• No declare(strict_types=1) anywhere in the 75 src/ files. That is precisely what would have
  caught the PostmarkBounce fallback swap at development time instead of in a v8 audit. A major is
  the moment, if it's ever going to happen.
• phpstan.neon is level 5 and phpstan/phpstan is pinned ^1.12 → 1.12.34, which the tool itself
  reports as two majors stale. With static analysis now the only credential-free gate, its level is
  the entire ceiling on what a fork PR can prove.

Sequencing note

#168's base is #167's branch, so it contains #167's six getter fixes and the getBounces() signature;
the v7.0.1 and v8.0.0 CHANGELOG entries both land on main with #168 if #167 isn't tagged first.
Stated as fact, no ask.

Coverage note

Six seats plus a devil's-advocate pass. None abstained. All six ran full passes with independent
execution — no seat deferred, and every load-bearing claim above was executed or pulled from a real
build, not read.

• php — request-changes. Executed everything in php:8.1/8.2/composer:2 containers with
  --network none. The live Postmark API was never called by any seat.
• architect — request-changes (mostly on #167's version digit, which is #167's review). Ran the
  variance tests and the mechanical undeclared-break sweep in Docker.
• circleci — approve-with-should-fix. Pulled real builds 678 and 704 and real commit statuses.
• product — fit: yes-with-concerns, advisory.
• em — bundled-but-defensible, advisory.
• cost — approve. Increase is real (1-2 executing jobs/push → 7) but lands on ~1 merge/year.
• devil — advisory. Downgraded the TemplateValidationResponse:197 item to out-of-scope,
  independently verified the credential-guard hole, and ruled that request-changes survives on that
  finding alone. Applied as written.

Two calibration notes. The pr-review-php seat is tuned for Postmark's Craft CMS 4 / Yii2 estate;
none of that applies to a standalone client SDK, so it was reviewed on the transferable axes — type
correctness, null handling, exception paths, composer supply chain, static analysis — with published-
library API compatibility and SemVer as the governing concern. And the usual Postmark tie-breaker
doesn't bite here: this touches no sending path, no message streams, no webhook processing, no
quota logic and no tenant isolation. The nearest thing to a production surface is
getDeliveryStatistics() returning real bounce counts for the first time, which is a customer-visible
deliverability signal and is correctly documented. That absence is exactly why a test-harness
credential guard is the thing gating this review: with no production blast radius competing for
attention, the strongest finding is the one that reproduces the fatal the release was written to kill.