Conversation
github-actions
Bot
added
documentation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds AI workflow/playbook and prompt documents, new Cursor rules, CI/CD workflow restructures, stronger static-analysis/tooling configs, Composer scripts and hook changes, several source refactors (client, middleware, caching, logging), and documentation updates for release and developer workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Builder as ClientBuilder
participant Client as HttpClient
participant Pipeline as MiddlewarePipeline
participant Adapter as HTTP Adapter
participant Logger as MongoDbLogger
Dev->>Builder: build() -> HttpClientInterface & AsyncHttpClientInterface
Builder->>Client: returns configured client (handler stack, headers)
Client->>Pipeline: request/async request with options
Pipeline->>Adapter: normalized options, middleware chain
Adapter-->>Client: Response / Promise
Client->>Logger: log request/response (via LoggingMiddleware helpers)
Logger-->>Dev: persisted log entry (optional)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoRelease 1.2.2: Refactor internals, enhance CI/CD, and implement AI workflow scaffolding
WalkthroughsDescription• Refactored cache payload reading and expiration parsing into separate methods for improved maintainability • Restructured ClientBuilder.build() to extract adapter and handler-stack creation into dedicated private methods • Simplified batch processing in HttpClient by extracting promise resolution and wrapping logic • Enhanced PHP-CS-Fixer configuration to focus on PHPDoc cleanup without overlapping with Pint • Expanded CI/CD workflows with security checks, lint matrix, dependency review, and coverage uploads • Added comprehensive AI workflow scaffolding: Claude commands, Cursor rules, and Antigravity/JetBrains prompts • Updated documentation, coding standards, and linting guides to reflect new tooling and validation contract • Bumped version to 1.2.2 and updated CHANGELOG with release notes Diagramflowchart LR
A["Source Code Refactoring"] -->|extract methods| B["Improved Maintainability"]
C["PHP-CS-Fixer Config"] -->|focus PHPDoc| D["Non-overlapping Tools"]
E["CI/CD Expansion"] -->|security, lint matrix, coverage| F["Enhanced Validation"]
G["AI Scaffolding"] -->|commands, rules, prompts| H["Documented Workflows"]
I["Documentation Updates"] -->|standards, guides| J["Aligned Guidance"]
B --> K["Version 1.2.2"]
D --> K
F --> K
H --> K
J --> K
File Changes1. .php-cs-fixer.dist.php
|
Code Review by Qodo
1. Duplicate middleware on rebuild
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+254
to
+286
| private function createDefaultAdapter(): TransportAdapterInterface | ||
| { | ||
| $guzzleOptions = $this->options; | ||
| $guzzleOptions['handler'] = $this->createHandlerStack($guzzleOptions); | ||
| $guzzleOptions['headers'] = $this->normalizeHeaders($guzzleOptions['headers'] ?? []); | ||
|
|
||
| $guzzle = new GuzzleClient($guzzleOptions); | ||
|
|
||
| return new GuzzleHttpClientAdapter($guzzle); | ||
| } | ||
|
|
||
| /** | ||
| * @param array<string, mixed> $guzzleOptions | ||
| */ | ||
| private function createHandlerStack(array &$guzzleOptions): HandlerStack | ||
| { | ||
| $userHandler = $guzzleOptions['handler'] ?? null; | ||
| unset($guzzleOptions['handler']); | ||
|
|
||
| if ($userHandler instanceof HandlerStack) { | ||
| $handlerStack = $userHandler; | ||
| } elseif (is_callable($userHandler)) { | ||
| $handlerStack = HandlerStack::create($userHandler); | ||
| } else { | ||
| $handlerStack = HandlerStack::create(); | ||
| } | ||
|
|
||
| if ($this->pipeline !== null) { | ||
| return $this->pipeline->buildHandlerStack($handlerStack); | ||
| } | ||
|
|
||
| return $handlerStack; | ||
| } |
There was a problem hiding this comment.
1. Duplicate middleware on rebuild 🐞 Bug ≡ Correctness
ClientBuilder::build() now reuses the same user-supplied HandlerStack object across multiple build() calls and re-runs MiddlewarePipeline::buildHandlerStack(), which pushes middleware onto that same stack again. This can lead to duplicated logging/retry/cache/interceptor layers (e.g., double logging, double retries) when a builder instance is reused.
Agent Prompt
### Issue description
`ClientBuilder::build()` can be called multiple times on the same builder. If the user provided a `GuzzleHttp\HandlerStack` via `withOption('handler', $stack)`, the builder currently reuses *the same stack object* and re-applies the middleware pipeline on every build, pushing duplicate middleware entries.
### Issue Context
- `createHandlerStack()` unsets `handler` from the *local* `$guzzleOptions` array only; the original `$this->options['handler']` remains set.
- When the handler is a `HandlerStack`, it’s an object reference, so repeated builds mutate the same instance.
### Fix Focus Areas
- src/Client/ClientBuilder.php[254-286]
- src/Middleware/MiddlewarePipeline.php[78-130]
### Implementation notes
Choose one:
1) **Clone** a user-supplied `HandlerStack` before passing it into `MiddlewarePipeline::buildHandlerStack()` (preferred, avoids mutating caller-owned stack and prevents duplication).
2) **Consume** the builder option (unset `$this->options['handler']`) after the first build, restoring prior “one-time consumption” behavior.
3) Add an internal guard to avoid pushing middleware that already exists on the stack (harder / more coupled to Guzzle internals).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
CI/CD
Chores