Skip to content

Bump database lib 7.3.4 - #14

Open
anurag6569201 wants to merge 1 commit into
qa/agent-appwrite-appwrite/pr-14-13439/basefrom
qa/agent-appwrite-appwrite/pr-14-13439/head
Open

anurag6569201 wants to merge 1 commit into
qa/agent-appwrite-appwrite/pr-14-13439/basefrom
qa/agent-appwrite-appwrite/pr-14-13439/head

Conversation

@anurag6569201

Copy link
Copy Markdown

What does this PR do?

(Provide a description of what this PR does and why it's needed.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

  • (Related PR or issue)

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

Source merge-base: a7e35dd376c704460e9239cd9dd158fc684e1b4a
Source head: 84aec17d403afa54341288b829b5e7074840a7a6

@shipwright-agent

Copy link
Copy Markdown

⛔ Shipwright · Blocked

Recommendation: do not merge PR #14 · Tier T3
Checks: 0 total · 0 needing attention

Next step: resolve the blocking findings before merge.

Findings (7)

  • CRITICAL The change from upsertDocument to upsertDocuments silently changes return-value semantics. · src/Appwrite/Platform/Workers/Executions.php:110
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • CRITICAL The test callback signature for upsertDocuments includes optional parameters ($batchSize, $onNext, $onError) and asserts $onNext is null. · tests/unit/Platform/Workers/ExecutionsTest.php:68
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • CRITICAL composer.lock downgrades plugin-api-version from 2.9.0 to 2.6.0 while simultaneously upgrading multiple packages. · composer.lock:8669
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The comment 'upsertDocuments without a callback skips the sequence fetch that upsertDocument always pays for' assumes the reader knows what a 'sequence fetch' is and why it matters · src/Appwrite/Platform/Workers/Executions.php:110
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The test method testUpsertsCompletedExecutionWithLogs now asserts on the internal callback parameter $onNext being null. · tests/unit/Platform/Workers/ExecutionsTest.php:75
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The dependency bump for appwrite/sdk-generator jumps from 4.5.0 to 4.7.6 (two minor versions) in a single lock update. · composer.lock:5525
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH Multiple utopia-php packages are upgraded in the same lock update (database 7.3.1->7.3.4, queue 1.5.1->1.5.2, usage 0.15.0->0.15.2, vcs 5.2.4->5.2.5). · composer.lock:3639
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Fireworks usage: 23,027 input · 1,004 output · 24,031 total tokens · $0.0057 · 17s · 0 fix iteration(s)

Open the Shipwright check for full evidence and the audit bundle. Use /shipwright rerun to verify again.

$this->create($dbForProject, $execution);
} else {
$dbForProject->upsertDocument('executions', $execution);
// upsertDocuments without a callback skips the sequence fetch that

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · CRITICAL

The change from upsertDocument to upsertDocuments silently changes return-value semantics.

Impact: The change from upsertDocument to upsertDocuments silently changes return-value semantics. The old path returned the upserted Document; the new path returns an int count and the code never reads it, so any caller or future code expecting the document will get a type change. More importantly, the comment claims skipping the sequence fetch is safe because 'nothing here reads the result back,' but this is an unverified…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

->willReturnCallback(function (string $collection, Document $execution) use (&$upserted): Document {
$upserted = $execution;
return $execution;
->method('upsertDocuments')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · CRITICAL

The test callback signature for upsertDocuments includes optional parameters ($batchSize, $onNext, $onError) and asserts $onNext is null.

Impact: The test callback signature for upsertDocuments includes optional parameters ($batchSize, $onNext, $onError) and asserts $onNext is null. This test now encodes an implementation detail of the database adapter rather than the worker's behavior. If utopia-php/database 7.3.4 changes the default callback behavior or batch semantics, this test will pass while production silently changes write behavior. The assertion 'No…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Comment thread composer.lock
"ext-fileinfo": "*"
},
"plugin-api-version": "2.9.0"
"plugin-api-version": "2.6.0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · CRITICAL

composer.lock downgrades plugin-api-version from 2.9.0 to 2.6.0 while simultaneously upgrading multiple packages.

Impact: composer.lock downgrades plugin-api-version from 2.9.0 to 2.6.0 while simultaneously upgrading multiple packages. This indicates the lock file was generated with an older or incompatible Composer version. A plugin-api downgrade can cause Composer to silently skip security-relevant plugins (e.g., audit, vulnerability scanning) or resolve dependencies differently in CI versus production, potentially shipping with unve…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

$this->create($dbForProject, $execution);
} else {
$dbForProject->upsertDocument('executions', $execution);
// upsertDocuments without a callback skips the sequence fetch that

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

The comment 'upsertDocuments without a callback skips the sequence fetch that upsertDocument always pays for' assumes the reader knows what a 'sequence fetch' is and why it matters

Impact: The comment 'upsertDocuments without a callback skips the sequence fetch that upsertDocument always pays for' assumes the reader knows what a 'sequence fetch' is and why it matters. A new maintainer cannot verify this claim without reading the utopia-php/database internals. The optimization is undocumented in the worker's contract and will be cargo-culted or removed by someone who doesn't understand the performance…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

array $executions,
int $batchSize = 0,
?callable $onNext = null,
?callable $onError = null

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

The test method testUpsertsCompletedExecutionWithLogs now asserts on the internal callback parameter $onNext being null.

Impact: The test method testUpsertsCompletedExecutionWithLogs now asserts on the internal callback parameter $onNext being null. This couples the worker test to the database library's private API shape. If the library adds a default callback or reorders parameters, the test breaks for reasons unrelated to the worker's behavior, creating maintenance noise and obscuring real regressions.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Comment thread composer.lock
{
"name": "appwrite/sdk-generator",
"version": "4.5.0",
"version": "4.7.6",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

The dependency bump for appwrite/sdk-generator jumps from 4.5.0 to 4.7.6 (two minor versions) in a single lock update.

Impact: The dependency bump for appwrite/sdk-generator jumps from 4.5.0 to 4.7.6 (two minor versions) in a single lock update. This is a dev dependency, but SDK generator output can be committed to the repository or used in release pipelines. A two-minor-version jump without an accompanying changelog or test evidence means generated SDK code could change behavior in ways not reviewed in this diff.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Comment thread composer.lock
{
"name": "utopia-php/database",
"version": "7.3.1",
"version": "7.3.4",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

Multiple utopia-php packages are upgraded in the same lock update (database 7.3.1->7.3.4, queue 1.5.1->1.5.2, usage 0.15.0->0.15.2, vcs 5.2.4->5.2.5).

Impact: Multiple utopia-php packages are upgraded in the same lock update (database 7.3.1->7.3.4, queue 1.5.1->1.5.2, usage 0.15.0->0.15.2, vcs 5.2.4->5.2.5). The database upgrade is directly relevant to the upsertDocuments change in this PR, but the queue and usage upgrades are unrelated and bundled together. Bundling unrelated dependency changes makes it impossible to bisect a production regression to a single…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant