fix: bind pinning provider fetch fallback to globalThis for browser compatibility#477
Merged
Conversation
…diagnosed Human verification items from 19.1-VERIFICATION.md executed via driven browser session. All 9 tests passed: upload, folder create/move/rename, bin delete/restore round-trip, logout/re-login lifecycle. Findings recorded in UAT Gaps for fix planning: - major: KuboProvider/PsaProvider unbound globalThis.fetch fallback throws Illegal invocation in browsers - external/dual pinning upload has never worked in a browser (Phase 21 surface, exact fix documented) - minor: bin rows show 0 B for deleted file size - cosmetic: delete dialog claims action cannot be undone despite bin Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 36431b6e6b11
KuboProvider and PsaProvider defaulted fetchFn to unbound globalThis.fetch and invoked it as this.fetchFn(url). Native browser fetch called with this set to the provider instance throws TypeError Illegal invocation, so external-only and dual-pin upload failed instantly at 0% in browsers. Node fetch is not this-sensitive, which is why unit and integration tests never caught it. Found during Phase 19.1 human UAT (see 19.1-UAT.md Gaps). Regression tests stub a this-sensitive fetch that mirrors the browser contract; verified to fail with the exact production error without the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 5fa84813c643
Entire-Checkpoint: 6bb75352d8d3
…orker STORAGE tab UX and the credential-encryption property verified in a real browser session. The pinning fix from 9789efa is e2e-verified: actual kubo-provider source pinned to the byo-ipfs-kubo Docker node with native fetch, while the pre-fix pattern reproduces Illegal invocation as a negative control. Remaining items (advisory badge, migration controls, in-app BYO upload wiring) are gated on a working local TEE worker: TEE_WORKER_URL defaults to port 3001 which mock-ipns-routing occupies, and TEE_WORKER_SECRET is unset. Details in 21-UAT.md Environment Blockers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 1db7735447d5
Local TEE worker now runs in Docker simulator mode (port 3002, epoch 1 key matches DB via deterministic HKDF seed), unblocking the previously env-blocked items. Connection test, migration controls, and external mode upload all verified live against the byo-ipfs-kubo node. Diagnosed gaps: - major: web never calls PATCH /vault/byo-status, advisory badge unreachable through product flow (rendering verified working) - minor: MigrationProgress polling never restarts after save when the previous job ended terminal; stale failed job shown as migrating - minor: migration stats double-count on API batch timeout (44+33>57) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 739dddf21e95
…staging infra Both human_verification items concerned external state and were verified remotely. The Phala CVM health check is obsolete: f270d84 intentionally retired the staging CVM in favor of a local Docker simulator on the VPS, and the replacement deployment is verified started via deploy run logs plus live API version match. GitHub staging env config verified present but now orphaned. Two minor gaps recorded: orphaned PHALA_* CI config and stale ENVIRONMENTS.md staging TEE docs. UAT debt audit now reports zero outstanding items. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 920410650ab4
…mulator PR #472 moved the staging TEE worker from a Phala Cloud CVM back to a local Docker Compose service in simulator mode, but the planning docs still described the CVM as the live staging deployment. Rework the ENVIRONMENTS.md staging TEE section around the simulator, move the CVM identity-preservation warning to the production section, replace the fictional TEE_ENABLED/TEE_PROVIDER/PHALA_API_URL env rows with the real TEE_WORKER_URL/TEE_WORKER_SECRET vars, and fix the codebase docs that claimed staging deploys to Phala. Phala Cloud CVM remains documented as the production design. Resolves the stale-docs gap from 35-UAT.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 34152199eb92
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 9b14fefbebc5
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
WalkthroughThis PR combines a browser compatibility fix in the SDK pinning providers (KuboProvider, PsaProvider) with comprehensive documentation updates reflecting the staging TEE infrastructure transition from Phala Cloud CVM to a local Docker simulator deployment. It resolves identified gaps from Phase 35 TEE migration and Phase 19.1 UAT, updating all planning documents to reflect the new deployment topology. ChangesTEE Infrastructure & SDK Fetch Binding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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 |
github-actions
Bot
added
release:sdk-core:fix
Contributor
Release Preview
Cascade Details
|
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #477 +/- ##
=======================================
Coverage 62.34% 62.34%
=======================================
Files 135 135
Lines 10157 10157
Branches 1081 1081
=======================================
Hits 6332 6332
Misses 3601 3601
Partials 224 224
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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
1 participant
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
KuboProviderandPsaProviderdefaulted to unboundglobalThis.fetchand invoked it asthis.fetchFn(...). Native browser fetch called withthisset to the provider instance throwsTypeError: Failed to execute 'fetch' on 'Window': Illegal invocation, which broke all uploads for accounts with external/dual pinning mode — instant 0% failure with no network request and no console output (the error was swallowed by a silent catch in the upload pipeline). Node's fetch is not this-sensitive, so no automated test ever caught it; the bug was latent since Phase 21. Fix:globalThis.fetch.bind(globalThis)in both providers.PinataProvideruses barefetch(...)calls and was never affected).Diagnosed gaps recorded for follow-up (not fixed here)
PATCH /vault/byo-status, so the advisory quota badge is unreachable through the product flow (major, 21-UAT)MigrationProgressstops polling permanently after a terminal job; failed jobs render as "migrating:" (minor, 21-UAT)PHALA_CLOUD_API_KEYsecret +PHALA_TEE_WORKER_URLvariable in the GitHub staging environment (minor, 35-UAT)Test plan
pnpm --filter @cipherbox/sdk-core test— including new browser this-binding regression tests, proven to fail pre-fixipfs pin lsconfirmed recursive pin)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Version Updates