Scope app-data/clientId unique indexes by thingtime; 400 instead of 500 on null search conditions - #90
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
🤖 PR Conflict Resolver: this PR conflicts with To enable AI conflict resolution, add the |
|
|
|
|
|
|
|
Conflicted files (as recorded by the merge step):
|
|
🤝 Merged Conflicted files:
Please review the merge commit before relying on it. |
github-actions
Bot
added
the
ai-rebase-in-progress
|
🧬 Rebased
The branch was published once with an exact force-with-lease. Please review the rewritten commits before relying on them. |
github-actions
Bot
removed
the
ai-rebase-in-progress
7fddf51 to
e019a18
Compare
|
🤖 Auto-resolve running — the conflict resolver started working on this PR at 14:27 UTC. Expected to finish around 14:35 UTC (resolutions typically land in 3-8 minutes; the job times out at 30). On success a merge commit resolving the conflicts is pushed to this branch and a result comment follows — no manual action is needed meanwhile. |
|
🤝 Merged Conflicted files:
Please review the merge commit before relying on it. |
github-actions
Bot
added
the
ai-rebase-in-progress
|
🤖 Auto-rebase running — the stack rebase started working on this PR at 09:27 UTC. Expected to finish around 09:42 UTC (most rebases still land in 5-15 minutes; the hosted-runner ceiling is 360). On success this branch is force-pushed onto its new base and a result comment follows — no manual action is needed meanwhile. |
2e52d5a to
221402a
Compare
github-actions
Bot
removed
the
ai-rebase-in-progress
|
🧬 Rebased
The branch was published once with an exact force-with-lease. Please review the rewritten commits before relying on them. |
|
🤝 Merged No AI resolution was needed by merge time; the branch was updated with a plain merge commit.
Please review the merge commit before relying on it. |
github-actions
Bot
added
the
ai-rebase-in-progress
|
🧬 Rebased
The branch was published once with an exact force-with-lease. Please review the rewritten commits before relying on them. |
github-actions
Bot
removed
the
ai-rebase-in-progress
d4b47d2 to
60ca2a8
Compare
|
|
🧹 Develop S3 preview removedThe PR-specific alias and every workflow-created develop deployment were removed when this PR closed. The ordinary generated Vercel Preview remains available on the shared development runtime. |
|
|
github-actions
Bot
added
the
ai-rebase-in-progress
PR #69 merged before these landed; both are fixed forward against main. 1. app-data / clientId unique indexes matched free-form data things (collections.ts). The (ownerId, crystal.appId, crystal.key) index filtered on crystal.appId+crystal.key existing but NOT on thingtime, while setAppData filters thingtime:'app-data'. So a plain thingtime:['data'] thing using 'appId'/'key' as ordinary data keys (sanitizeDataCrystal allows both) squats the unique slot: app writes miss it, insertOne 11000s all 3 attempts, and the caller gets a permanent 503. The likelier hit is user-facing — two ordinary data things by one owner sharing those values 409 each other via the duplicate-key mapping in things.ts. The crystal.clientId index had the same exists-only shape. Both are now scoped by thingtime, the same fix already applied to the reaction index (targetId: {$type:'string'}) for this exact collision class. Narrowing clientId keeps its structural-uniqueness property intact because every lookup in apps/apps.ts already filters thingtime:'app' — a data thing could never answer one. Deployed DBs already carry these indexes under auto-generated names, so both move to createIndexReplacing with explicit names + the legacy names, reusing the existing 85/86 conflict path. Verified against a real mongod: reproduced the old 11000 collisions, ran the in-place upgrade, confirmed the legacy names are gone, that app-data and data things now coexist, that genuine duplicates (same user/app/key, and duplicate clientId) are still blocked, that a different owner is still allowed, and that a re-run is a no-op with no drop. ensureIndexes() then migrated the local dev DB in place from the old indexes. 2. conditions:[null] returned 500 instead of 400 (search.ts). isGroup(null) is false, so buildCondition(null) dereferenced null.field; the API catch-all rethrows non-Response errors. Every other malformed shape already 400s, so this was a single missed guard. Rate limiting runs first, so it was never a DoS lever — a wrong status code and error-log noise. Verified by driving the real searchThings(): null / undefined / nested null in a group all threw TypeError before and all return 400 now; string, number, array and {} were and remain 400; a valid condition still queries normally. tsc --noEmit unchanged vs main (136 -> 136; zero new type errors). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Refreshed by the rebase-pr-stacks workflow: https://github.com/lopugit/thingtime/actions/runs/32015271273
7095d8f to
7ced028
Compare
github-actions
Bot
removed
the
ai-rebase-in-progress
|
🧬 Rebased
The branch was published once with an exact force-with-lease. Please review the rewritten commits before relying on them. |
|
🤖 Promotion conflict resolution was queued automatically for The trusted worker will reconstruct, verify, publish, and attest the review branch; no manual branch update is needed. |
|
✅ The trusted promotion worker opened #296 for Review the exact resolved paths and immutable snapshot in the promotion PR comment. Workflow run. |
|
🤖 Verified automatic promotion resolution for This marker is inert until the live remote branch equals the attested head. Workflow run. |
Thingtime-Promotion-Source-PR: 90 Thingtime-Promotion-Base-Ref: main Thingtime-Promotion-Base-SHA: fd970e9 Thingtime-Promotion-Branch: promote/pr-90-fix-appdata-index-search-null-guard--to-main Thingtime-Promotion-Source-Tip-SHA: 6a026ec Thingtime-Promotion-Source-Start-SHA: 08699b9 Thingtime-Promotion-Source-End-SHA: 6a026ec Thingtime-Promotion-Source-Lineage: verified Thingtime-Promotion-Plan-Hash: 52e75f7eab1671a81c0c8c21e1a6cc0348a9bc60ca7d5a7d29eae8c32b28ee16 Thingtime-Promotion-Patch-ID: 432937c84e53ae84298e7dd9a94a51dbe0cfefa4 Resolved by the promotion worker: https://github.com/lopugit/thingtime/actions/runs/32030650552
… by thingtime; 400 instead of 500 on null search conditions (#90)
… by thingtime; 400 instead of 500 on null search conditions (#90)
… by thingtime; 400 instead of 500 on null search conditions (#90)
… by thingtime; 400 instead of 500 on null search conditions (#90)
…search-null-guard--to-main [Promote] Scope app-data/clientId unique indexes by thingtime; 400 instead of 500 on null search conditions (#90)
1 participant
Summary
This is the single combined PR for the two confirmed PR #69 review findings. It supersedes #83 and #86.
crystal.clientIdunique index tothingtime: 'app'.(ownerId, crystal.appId, crystal.key)unique index tothingtime: 'app-data'.createIndexReplacing.conditions: [null].things-search-null-conditionthings-data-app-shaped-createthings-data-app-shaped-duplicateWhy
Free-form data Things may legitimately carry
clientId,appId, andkeyinsidecrystal. Exists-only partial filters made those ordinary data Things participate in unrelated app-platform uniqueness constraints. That could make two ordinary data Things collide, or let one data Thing wedge an app-data write into repeated duplicate-key failures.The search parser separately dereferenced
null.field, turning malformed client input into an unhandled 500.The new partial filters use the same
thingtimediscriminators already stamped and queried by the app and app-data write paths, so genuine app/app-data uniqueness remains enforced while unrelated data Things no longer collide.Consolidation
developinto this branch.Verification
auth+things: 51/51 pass, including all three restored regression cases.🤖 Consolidated with Codex