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

These are locally-verified numbers against a fully-configured environment, not CI's. Raised in review and worth stating plainly: CircleCI's integration job at d4a6a698 reported Tests: 87, Errors: 4, Skipped: 31. The 92-vs-87 count is the full suite versus --testsuite integration, and the 20→4 error reduction is the fix working — but the table as originally written read as though CI agreed, and it visibly didn't.

Credential guard — wider than first described (review item 1)

The guard hole ac-bgelfius found was real, and reproducing it turned up two things the remedy as written would not have closed:

• The cited reproduction fatals before any setUp() runs. testClientCanGetBounce dies in setUpBeforeClass, which calls PostmarkClientSuppressionsTest::tearDownAfterClass() — a static cross-class cleanup that constructs a client. No instance guard can reach it. PostmarkClientTemplatesTest has the same shape in its own static hooks. Both now return early when the token is absent.
• The scope is 16 classes, not 5. Guarding only the named five left 58 TypeError fatals under partial credentials. Every class that builds a client from an unguarded token had the same hole; all now guard the token they actually use.

Measured across three scenarios:

environment	before	after
no credentials at all	skips everything	skips everything (unchanged)
partial credentials	58 TypeError fatals, 25 skips	0 fatals, 60 skips
unit suite	green	green

The 31 errors remaining in the partial case are auth failures from a deliberately-invalid token — the correct outcome, and distinguishable from a defect, which is the point of the release.

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.

[ewood-ac]

All eight should-fixes addressed at a8b7b71. Item 1 was right, and reproducing it turned up two things that change the remedy — worth reading before the re-review.

item	disposition
1 credential guard hole	Fixed — wider than described, see below
2 Guzzle 8 reclassification filed under ### Added	Moved to ### Changed with BREAKING
3 Guzzle floor rise described under Added	Moved to ### Changed
4 integration-tests exits 0 having run nothing	Fails on trunk, fork PRs still skip
5 PostmarkBounce note half wrong	Corrected
6 results table reads as if CI agrees	Caveated in the description
7 @file_get_contents() hides the reason	error_get_last() folded in
8 no "Upgrading from v7" section	Added

Item 1 — two corrections to the mechanism

Your remedy does not close the reproduction you cited. testClientCanGetBounce fatals in setUpBeforeClass, which calls PostmarkClientSuppressionsTest::tearDownAfterClass() — a static cross-class cleanup that constructs a client. That runs before any instance setUp(), so no parent::setUp() chain or requireKeys() can reach it. I confirmed by reproducing: with WRITE_ACCOUNT_TOKEN set and WRITE_TEST_SERVER_TOKEN absent, the setUp fix alone still gave Errors: 1. PostmarkClientTemplatesTest has the same shape in its own static hooks. Both now return early when the token is absent — cleanup that cannot run is a no-op, not a failure.

The scope is 16 classes, not 5. Fixing only the named five left 58 TypeError fatals under partial credentials. Every class that builds a client from a token nothing guards has the identical hole. All now guard the token they actually use:

WRITE_ACCOUNT_TOKEN                    DataRemoval, Domain, SenderSignature, Servers, MessageStreams
WRITE_TEST_SERVER_TOKEN                Email, EmailsAsStringOrArray, Templates, RuleTrigger, Server,
                                       Suppressions, Webhooks
READ_SELENIUM_TEST_SERVER_TOKEN        InboundMessage, OutboundMessage, Bounce (+ WRITE_TEST_SERVER_TOKEN)
READ_LINK_TRACKING_TEST_SERVER_TOKEN   ClickClientStatistics
READ_SELENIUM_OPEN_TRACKING_TOKEN      Statistics — already correct, it guarded the token it used

Your token mapping for the five was exactly right, including that Bounce needs both.

One small correction: two of the five already chained parent::setUp() (Bounce, Templates) at d4a6a698.

Measured:

environment	before	after
no credentials	skips everything	unchanged
partial credentials	58 TypeError fatals, 25 skips	0 fatals, 60 skips
unit suite	green	green

The 31 errors left in the partial case are auth failures from a deliberately-invalid token — the correct outcome, and exactly the distinction this release is about.

A mistake I made and caught before committing

My first pass at the static-hook guard matched too broadly and put the early return inside eight test methods as well as the two hooks. An early return in a test method reports a pass with zero assertions — the same silent-green failure this PR exists to remove, introduced by the fix for it. Reverted to the two static hooks only, and the comment now says why so nobody widens it.

On the CI coverage point

Your observation that php81 being first in the requires chain meant php82/83/84 never executed for ten months is the strongest argument in the review, and you're right it wasn't in the PR body. It's the argument for never chaining a version matrix again, and it's now in the description.

Not done

The nit about phpunit.xml.dist — the unit suite being a single hardcoded <file>, so php82/83/84/85 each run ~5 assertions and the full suite runs on 8.2 only. That's a real per-version depth loss and I've left it: widening the unit suite means deciding which of the 17 live-API classes can run credential-free, which is a bigger change than this release should absorb. Worth its own ticket, and worth saying rather than letting the matrix look deeper than it is.

[ac-bgelfius]

Approve at a8b7b712. All four items are addressed, and the credential guard was closed more completely than I asked. One residual on the new CI guard — it narrows the silent-green hole rather than closing it.

Item 1 — the credential guard, closed and swept

I named five classes. The sweep went wider and I verified it mechanically, with a control at the old head so an empty result means something:

ref	setUp() overrides missing parent::setUp()
d4a6a698 (old)	6 — Bounce, Email, EmailsAsStringOrArray, Statistics, Templates, Base
a8b7b712 (head)	1 — PostmarkClientBaseTest only

The remaining one is the class that defines the guard, so it has no parent guard to chain. Correct exclusion, not a miss — and it's a sixth file I hadn't spotted.

And the per-key half is right, which is the part that actually closes it. Chaining parent::setUp() alone wouldn't have, because hasAnyCredentials() passes on any one of six tokens:

PostmarkClientBounceTest                 requireKeys('READ_SELENIUM_TEST_SERVER_TOKEN', 'WRITE_TEST_SERVER_TOKEN')
PostmarkClientEmailTest                  requireKeys('WRITE_TEST_SERVER_TOKEN')
PostmarkClientEmailsAsStringOrArrayTest  requireKeys('WRITE_TEST_SERVER_TOKEN')
PostmarkClientTemplatesTest              requireKeys('WRITE_TEST_SERVER_TOKEN')

You corrected me on the bounce class. I said it needed READ_SELENIUM_TEST_SERVER_TOKEN; it builds clients from both — READ_SELENIUM_TEST_SERVER_TOKEN at :47, :67, :149, :163 and WRITE_TEST_SERVER_TOKEN at :84. Guarding both is the right answer and my recommendation was one token short.

Items 2 and 3 — filed where they'll be read

The Guzzle 8 reclassification is now CHANGELOG.md:69 under ### Changed, marked BREAKING (behaviour, not signature) — the right qualifier, since no signature moves — with the old→new exception mapping table. It also appears as a row in the new "### Upgrading from v7 — read this first" section at :49. Two prominent placements for the release's largest blast radius is the correct weighting.

The Guzzle floor is at :87 under ### Changed, leading with the consumer consequence ("can block composer update") rather than the security rationale. That's the ordering a consumer needs.

Item 4 — better, and one hole left

The new guard is the right idea and the comment explains the asymmetry well: fork PRs legitimately have no credentials because CircleCI withholds project env vars from them, so skipping there must stay green, while on trunk a missing token is a configuration fault. Tying it to CIRCLE_PR_NUMBER is correct for that — that variable is populated for forked PRs specifically.

But it checks one token as a proxy for all of them. The guard tests WRITE_ACCOUNT_TOKEN. The suite now requires, per the requireKeys() calls above, at least WRITE_TEST_SERVER_TOKEN, READ_SELENIUM_TEST_SERVER_TOKEN and READ_SELENIUM_OPEN_TRACKING_TOKEN as well. So a trunk build with WRITE_ACCOUNT_TOKEN set and any of the others missing passes the guard, and those classes skip — green, with a subset of the suite silently not running.

That's the same partially-configured-environment shape as item 1, one layer out: the CI guard has its own hasAnyCredentials() problem. Narrowed a lot — a fully unconfigured trunk build now fails loudly, which is the important case — but not closed.

Cheapest close is to check the set the suite actually names rather than one representative:

for v in WRITE_ACCOUNT_TOKEN WRITE_TEST_SERVER_TOKEN READ_SELENIUM_TEST_SERVER_TOKEN READ_SELENIUM_OPEN_TRACKING_TOKEN; do
  [ -n "$(eval echo \"\$$v\")" ] || { echo "$v unset on a trunk build"; exit 1; }
done

The more durable version asserts a floor on tests actually executed, so adding a class with a new token requirement can't reintroduce this — but that needs PHPUnit output parsing and is a bigger change than this release wants.

Unchanged from my last review

Everything I verified then still stands and is worth restating for whoever lands this: the PostmarkBounceSummary.php:15 FirstOpen/Count bug means getDeliveryStatistics() has reported zero for every bounce category in every released version, and the CHANGELOG is right to warn that dashboards calibrated on that zero will start seeing real numbers. The undeclared-break sweep came back clean — the only breaks are the three declared PostmarkAttachment factories. Guzzle 7 and 8 are both genuinely supported. And the pipeline rework is a correctness fix, not a wall-clock one: php82/php83/php84 had not executed at all for roughly ten months because php81 is first in the requires chain and has been red since 2025-10-24.

That last point is still the strongest argument in the PR and still deserves to be in the body rather than only in a review thread — it's the case for never chaining a version matrix again.

I ran no tests; the PHPStan-clean-on-both-Guzzles and 85-class load checks are as previously reported.