Repository navigation
Conversation
Introduce system library
Update composer description
feat(system): add method to check for specific architecture
fix enum class properties to be non abstract
feat(system): get enum of arch
…ion-suggest fix(telemetry): make ext-protobuf a suggestion, not a requirement
chore: stop committing package lock files
chore: stop committing package lock files
chore: stop committing package lock files
chore: stop committing package lock files
feat(system): add cgroup-aware memory capacity method
…trics fix(telemetry): omit empty metrics from OTLP exports
…ad-pooled-connection fix(telemetry): retry an export once when the pooled connection is reset mid-request
…stale-session fix(smtp): let a holder reuse a connection the server may have closed
perf(telemetry, circuit-breaker): record less per call on the cache path
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…the root The root replaces every absorbed package, which hides the package's own require block from the solver: a third-party dependency a package declares only resolves when the root declares it too. validate now fails for any non-php, non-ext, non-utopia-php requirement missing from the root require, and the RFC says so. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Step A of rfc/monorepo.md for the four leaves imported in the preceding absorb commits: sources flattened to src/, test namespaces renamed to Utopia\<Ns>\Tests\, service-bound tests under tests/E2E, standalone phpstan.neon, Appwrite's Pint style applied. The root drops utopia-php/telemetry and utopia-php/system from require (psr7 and smtp were only transitive) and replaces all four; their third-party requirements are hoisted into the root require: nyholm/psr7, open-telemetry/exporter-otlp, open-telemetry/sdk, symfony/http-client (telemetry), psr/http-factory, psr/http-message (psr7). The lock loses the four packages and nothing else moves. telemetry: src/Telemetry -> src; tests/Telemetry -> tests, Tests\Telemetry -> Utopia\Telemetry\Tests; TransportIntegrationTest and MockOtlpServer (bind a Swoole OTLP server on loopback) -> tests/E2E with a test:e2e script, no compose file. PHPStan level max, no baseline. rector.php made standalone (it required the old monorepo's ../../rector.php, which exists neither here nor on the mirror) and the PHPUnit set dropped (its rules are bonded to vendor/composer/installed.json, which a package has none of); Rector changed one file. system: src/System/System.php -> src/System.php; tests/System -> tests, Utopia\Tests -> Utopia\System\Tests; testGetEnv sets its own env instead of relying on phpunit.xml so the root packages suite can run it. New phpstan.neon at level 5 over src and tests, no baseline. No rector.php, none before. psr7: src/Psr7 -> src; tests/Psr7 -> tests, Utopia\Tests\Psr7 -> Utopia\Psr7\Tests. phpstan.neon standalone at its level 10 over src and tests, no baseline. rector.php made standalone as above; Rector changed nothing. smtp: tests/Unit -> tests, Utopia\SMTP\Tests\Unit -> Utopia\SMTP\Tests; tests/E2E and the Mailpit compose file unchanged. phpstan.neon standalone at level max over src and tests, no baseline. rector.php made standalone as above; Rector added @see test annotations to ten classes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| $this->assertSame('GET', Method::GET); | ||
| $this->assertSame('POST', Method::POST); | ||
| $this->assertSame('PUT', Method::PUT); | ||
| $this->assertSame('PATCH', Method::PATCH); | ||
| $this->assertSame('DELETE', Method::DELETE); | ||
| $this->assertSame('HEAD', Method::HEAD); | ||
| $this->assertSame('OPTIONS', Method::OPTIONS); | ||
| $this->assertSame('CONNECT', Method::CONNECT); | ||
| $this->assertSame('TRACE', Method::TRACE); |
There was a problem hiding this comment.
These assertions duplicate every method constant's literal value instead of testing behavior that uses the constants. They therefore provide no behavioral signal and fail only when the implementation and its mirror drift apart. The same pattern appears in Request/FactoryTest.php, which pins the private utopia- boundary prefix and concrete MultipartBody class rather than validating only the emitted multipart contract. This violates the repository directive not to mirror source code or configuration in assertions and must be corrected before merging.
Context Used: Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/psr7/tests/ConstantsTest.php
Line: 16-24
Comment:
**Tests mirror source literals**
These assertions duplicate every method constant's literal value instead of testing behavior that uses the constants. They therefore provide no behavioral signal and fail only when the implementation and its mirror drift apart. The same pattern appears in `Request/FactoryTest.php`, which pins the private `utopia-` boundary prefix and concrete `MultipartBody` class rather than validating only the emitted multipart contract. This violates the repository directive not to mirror source code or configuration in assertions and must be corrected before merging.
**Context Used:** Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
These tests are imported from the library unchanged; an absorb moves a package without rewriting it, so its mirror history stays a pure relocation. Test quality in psr7 and telemetry is library work and belongs in its own change against packages/psr7 and packages/telemetry, where it also releases through bin/monorepo release.
There was a problem hiding this comment.
That context is fair. An absorb commit should preserve the imported package history and remain a pure relocation; rewriting its tests would mix package-quality work into the migration. I agree this is not an actionable finding for PR #13721. The assertion-quality concerns belong in a follow-up change against packages/psr7 (and packages/telemetry where applicable), with its own release. I’m withdrawing this as a merge blocker for the absorb.
| public function testConstructorParsesEndpointCorrectly(): void | ||
| { | ||
| $transport = new Swoole('https://otel.example.com:4318/v1/metrics?foo=bar'); | ||
|
|
||
| $this->assertSame(ContentTypes::PROTOBUF, $transport->contentType()); |
There was a problem hiding this comment.
This test constructs an endpoint but checks only the unrelated contentType() getter. Broken host, port, path, query, or TLS parsing would leave it green. The custom-header, timeout, pool-size, socket-buffer, query-string, and default-port tests repeat this no-op pattern. This violates the repository directive to test observable behavior rather than mirror configuration and must be corrected before merging; these cases should observe actual requests or connections.
Context Used: Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/telemetry/tests/Adapter/OpenTelemetry/Swoole/TransportTest.php
Line: 47-51
Comment:
**Test does not parse endpoint**
This test constructs an endpoint but checks only the unrelated `contentType()` getter. Broken host, port, path, query, or TLS parsing would leave it green. The custom-header, timeout, pool-size, socket-buffer, query-string, and default-port tests repeat this no-op pattern. This violates the repository directive to test observable behavior rather than mirror configuration and must be corrected before merging; these cases should observe actual requests or connections.
**Context Used:** Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
✨ Benchmark resultsComparing
Per-scenario breakdown & investigation detailsMetrics below reflect the current branch (after). Δ P95 compares against the base.
Top API waits (after)
|
Summary
Phase 2 batch, four dependency-free leaves, each imported from its mirror with history (
bin/monorepo absorb, one subtree merge plus a plumbing commit per package) and reshaped to the standard layout (RFC step A, non-breaking):src/Telemetryflattened tosrc/src/Systemflattenedsrc/Psr7flattenedsrc/Tests\\Telemetry→Utopia\\Telemetry\\TestsUtopia\\Tests→Utopia\\System\\TestsUtopia\\Tests\\Psr7→Utopia\\Psr7\\Tests…\\Tests\\Unit→…\\Teststests/E2ENo baselines were needed. Rector configs kept where the mirror had one, made standalone (they used to
requirethe old monorepo's root config, which exists nowhere any more) and withphpunitCodeQualitydropped because it needs a package-localinstalled.json; smtp'srectorPresetalso dropped, since it annotated source classes with their test class names.Two things this batch settles for every later package
replaceat scale. Between them these four are required by 14 vendored packages (telemetry by nine). With the rootreplace-ing them, the lock drops all four and nothing else moves; the dependants resolve frompackages/.psr7andsmtpwere never in the rootrequireat all, only transitive, andreplacecovers that too.replacehides a package's ownrequirefrom the solver, its third-party dependencies must be declared by the root or they vanish from the lock. Six were hoisted here (nyholm/psr7,open-telemetry/exporter-otlp,open-telemetry/sdk,symfony/http-client,psr/http-factory,psr/http-message), andbin/monorepo validatenow fails when a package requirement is missing from the root.Telemetry drift, checked
Appwrite ran 0.4.10; the mirror head is one merge past the 0.4.11 tag. Since 0.4.8 the only public-surface change is an added exporter wrapper class, nothing removed or re-signed, so the nine dependants are unaffected.
Known local-only failure
bin/monorepo test telemetry's e2e tier segfaults on macOS (Swoole 6 + PHP 8.5) in one retry-timing test. Reproduced on the untouched import commit, so not introduced here; the unit tier passes. Let the Linux runner decide.Merging
Merge with a merge commit, not a squash: the four subtree annotations are what the split continues from.
Verification
bin/monorepo validate: all packages valid; the new hoisting rule fires when a hoisted line is removedbin/monorepo checkclean for all four;testgreen for system (33), psr7 (47), smtp (28 unit + 28 e2e); telemetry unit 24 greenvendor/bin/phpunit --testsuite packages: 450 tests, 4 skippedcomposer validatevalid;dump-autoloadwithout PSR-4 warnings🤖 Generated with Claude Code