[MrBeldum]

Summary

Fixes #254.

DeleteContext read filter["id"] unchecked and asserted id.(string) (and the same for pk). A filter without a string id, such as {"deleted_at": nil} or {"id": 42}, panicked.

Validate with comma-ok before creating the container client, and return a clear error instead of panicking.

Test plan

• go test ./storage/ -run TestCosmosDBDeleteContextRejectsMissingOrNonStringID

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6aecb772-7a96-4795-9264-df48565ab790

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Lutherwaves]

praise: Validating before NewContainer is a nice touch, and the table test covers the cases from #254. I checked that it fails on the old code.

issue (non-blocking): #254 says UpdateContext already handles this, but it only checks that the key exists, not its type. UpdateContext still does unchecked pk.(string) / id.(string) (cosmosdb.go:394,397), and CreateContext does pk.(string) (:219,222), so a non-string id/pk panics there too. Fine as a follow-up, but worth an issue so this class of bug is fully closed.

[Lutherwaves]

Thanks for your contribution!

[MrBeldum]

Addressed in e9b0b0f — verified just now:

• Redundant guard dropped: removed the now-duplicate len(filter) == 0 guard; the !ok check covers it.
• Constant messages: switched to errors.New(...) for the three no-verb messages.
• Unchecked assertions in UpdateContext/CreateContext: covered by fix(storage/cosmosdb): reject non-string create/update ids without panic #276, which routes both through nonEmptyString validation — so no separate follow-up issue was opened.

Validation: go test ./storage/ -run 'TestCosmosDB.*Delete|TestDelete' passes; gofmt -l clean on the changed files.

These changes were prepared with AI assistance and validated by running the package tests; not personally reviewed/tested by Daniel.

[MrBeldum]

Resolved the merge conflict with upstream main (merge commit 4cb056a). The conflict was in DeleteContext: kept this PR's validated string id (no bare type assertion) and kept upstream's session-token recording from DeleteItem. Full storage test suite passes locally; CI is the gate for the new head.

Disclosure: this conflict resolution was assisted by AI, reviewed against the PR's intent before pushing.