fix(desktop): align file upload IPNS sequence and verify file pointers#446
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughAdds a helper to compute per-file IPNS publish sequences and integrates it into FUSE publish flows to skip resolving sequence on first publish. Adds a file-pointer verifier script and updates desktop e2e tests to use randomized files and poll the verifier. Changes
Sequence Diagram(s)sequenceDiagram
participant TestClient as Test Client
participant Verifier as verify-filepointer
participant AuthAPI as Auth API
participant VaultSvc as Vault Service
participant IPFS as IPFS/IPNS
TestClient->>Verifier: run(file-name, expected-content, api-url, email, secret)
Verifier->>AuthAPI: POST /auth/test-login {email, secret}
AuthAPI-->>Verifier: {accessToken, privateKeyHex}
Verifier->>VaultSvc: GET /vault (Authorization: Bearer token)
VaultSvc-->>Verifier: {rootIpnsName}
Verifier->>IPFS: resolve(rootIpnsName) -> root folder metadata
Verifier->>IPFS: loadVaultKeyBlob(root) / loadFolderMetadata(root)
Verifier->>IPFS: find child by name -> fileMetaIpnsName
Verifier->>IPFS: resolve(fileMetaIpnsName) -> file CID/metadata
alt expected-content provided
Verifier->>IPFS: download(file CID)
IPFS-->>Verifier: encrypted content
Verifier->>Verifier: decrypt using privateKeyHex
Verifier->>Verifier: compare plaintext == expected-content
end
Verifier-->>TestClient: JSON report {rootIpns, fileCid, contentVerified}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Release Preview
Cascade Details
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the desktop FUSE upload flow to handle “first publish” IPNS sequence behavior explicitly and extends the desktop round-trip E2E scripts with a FilePointer/per-file IPNS verification step (via a new sdk-core verifier script).
Changes:
- Add
next_file_publish_sequence()helper and use it to compute per-file IPNS publish sequence numbers without resolving IPNS on first publish. - Enhance desktop E2E round-trip scripts (bash + PowerShell) to verify FilePointer presence and per-file IPNS metadata resolution/content round-trip.
- Add a Node verifier script (
packages/sdk-core/scripts/verify-filepointer.mjs) that authenticates viatest-login, loads folder metadata, resolves per-file metadata, and optionally verifies decrypted content.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/desktop-e2e/scripts/test-round-trip.sh | Adds repo-root detection, builds verifier runtime if needed, writes randomized test file, and verifies FilePointer + per-file metadata/content. |
| tests/desktop-e2e/scripts/test-round-trip.ps1 | Windows equivalent of the round-trip enhancements, including verifier runtime build and repeated verification attempts. |
| packages/sdk-core/scripts/verify-filepointer.mjs | New script to authenticate, load root folder metadata, find a FilePointer by name, resolve per-file metadata from IPNS, and optionally verify content. |
| crates/fuse/src/platform/windows/operations.rs | Uses the new sequence helper and skips sequence resolution on first per-file publish. |
| crates/fuse/src/operations.rs | Same per-file publish sequence change for the non-Windows implementation. |
| crates/fuse/src/lib.rs | Introduces next_file_publish_sequence() and adds unit tests for it. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #446 +/- ##
==========================================
+ Coverage 62.35% 71.92% +9.57%
==========================================
Files 135 114 -21
Lines 10088 7171 -2917
Branches 1053 1055 +2
==========================================
- Hits 6290 5158 -1132
+ Misses 3580 1795 -1785
Partials 218 218
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Owner
Author
|
@coderabbitai please review |
…e IPNS sequence handling
Agent-Logs-Url: https://github.com/FSM1/cipher-box/sessions/267838ae-5dc0-42ad-8277-64bbb23fc457 Co-authored-by: FSM1 <12774278+FSM1@users.noreply.github.com>
FSM1
force-pushed
the
fix/desktop-new-file-upload-ipns-sequence
branch
from
April 2, 2026 13:13
719ddef to
0432540
Compare
FSM1
enabled auto-merge (squash)
April 2, 2026 13:14
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/fuse/src/lib.rs (1)
124-135: Narrow this helper to crate visibility.
next_file_publish_sequenceis only used from internal modules, so keeping itpubunnecessarily expands the crate's public API surface.♻️ Proposed fix
-pub fn next_file_publish_sequence( +pub(crate) fn next_file_publish_sequence(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fuse/src/lib.rs` around lines 124 - 135, The function next_file_publish_sequence is unnecessarily public; change its visibility to crate-level (e.g., make it pub(crate) or remove pub if used only within the crate) so it no longer expands the public API surface, leaving the implementation and return type intact and ensuring internal modules can still call next_file_publish_sequence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sdk-core/scripts/verify-filepointer.mjs`:
- Around line 28-45: The script currently requires --secret; change logic in the
argument-parsing block that returns { apiUrl, secret, email, fileName,
expectedContent } so that secret is optional and falls back to
process.env.TEST_SECRET (i.e., set secret = values.get('secret') ||
process.env.TEST_SECRET), remove --secret from the required-arguments check and
from the usage error string, and keep the CLI flag as an override; then update
callers (tests/e2e-desktop/scripts/* wrappers) to stop passing --secret and rely
on TEST_SECRET in the environment instead.
In `@tests/desktop-e2e/scripts/test-round-trip.ps1`:
- Around line 45-64: The early-return guard in Ensure-VerifierRuntime only
checks $sdkCoreDist and $apiClientDist but must verify all four build outputs;
update Ensure-VerifierRuntime to define and test paths for the missing dist
files (packages/crypto/dist/index.mjs and packages/core/dist/index.mjs) and
change the if condition to require all four Test-Path results before returning
early, and make the equivalent change in the shell counterpart
test-round-trip.sh so both scripts check the same four prerequisites.
---
Nitpick comments:
In `@crates/fuse/src/lib.rs`:
- Around line 124-135: The function next_file_publish_sequence is unnecessarily
public; change its visibility to crate-level (e.g., make it pub(crate) or remove
pub if used only within the crate) so it no longer expands the public API
surface, leaving the implementation and return type intact and ensuring internal
modules can still call next_file_publish_sequence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7961481b-5cdd-4bc3-a17e-98e2bd11e5c8
📒 Files selected for processing (7)
crates/fuse/src/lib.rscrates/fuse/src/operations.rscrates/fuse/src/platform/windows/operations.rspackages/sdk-core/scripts/verify-filepointer.mjsrelease-please-config.jsontests/desktop-e2e/scripts/test-round-trip.ps1tests/desktop-e2e/scripts/test-round-trip.sh
Comment on lines
+28
to
+45
| const apiUrl = values.get('api-url'); | ||
| const secret = values.get('secret'); | ||
| const email = values.get('email'); | ||
| const fileName = values.get('file-name'); | ||
|
|
||
| if (!apiUrl || !secret || !email || !fileName) { | ||
| throw new Error( | ||
| 'Usage: verify-filepointer.mjs --api-url <url> --secret <secret> --email <email> --file-name <name> [--expected-content <text>]' | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| apiUrl, | ||
| secret, | ||
| email, | ||
| fileName, | ||
| expectedContent: values.get('expected-content'), | ||
| }; |
There was a problem hiding this comment.
Don’t make the test-login secret mandatory on argv.
Requiring --secret here forces both round-trip wrappers to put TEST_SECRET in process arguments. Fall back to process.env.TEST_SECRET, keep the flag as an optional override, and then drop --secret from the shell/PowerShell callers.
🔐 Proposed fix
- const secret = values.get('secret');
+ const secret = values.get('secret') ?? process.env.TEST_SECRET;
const email = values.get('email');
const fileName = values.get('file-name');
- if (!apiUrl || !secret || !email || !fileName) {
+ if (!apiUrl || !email || !fileName || !secret) {
throw new Error(
- 'Usage: verify-filepointer.mjs --api-url <url> --secret <secret> --email <email> --file-name <name> [--expected-content <text>]'
+ 'Usage: verify-filepointer.mjs --api-url <url> --email <email> --file-name <name> [--secret <secret>] [--expected-content <text>] (or set TEST_SECRET)'
);
}Based on learnings In PowerShell and Bash test scripts for tests/e2e-desktop/scripts/, secrets (like TEST_SECRET) should be passed via environment variables, NOT as command-line parameters, to avoid exposure in process listings and shell history.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk-core/scripts/verify-filepointer.mjs` around lines 28 - 45, The
script currently requires --secret; change logic in the argument-parsing block
that returns { apiUrl, secret, email, fileName, expectedContent } so that secret
is optional and falls back to process.env.TEST_SECRET (i.e., set secret =
values.get('secret') || process.env.TEST_SECRET), remove --secret from the
required-arguments check and from the usage error string, and keep the CLI flag
as an override; then update callers (tests/e2e-desktop/scripts/* wrappers) to
stop passing --secret and rely on TEST_SECRET in the environment instead.
Comment on lines
+45
to
+64
| function Ensure-VerifierRuntime { | ||
| $sdkCoreDist = Join-Path $RepoRoot "packages/sdk-core/dist/index.mjs" | ||
| $apiClientDist = Join-Path $RepoRoot "packages/api-client/dist/index.mjs" | ||
|
|
||
| if ((Test-Path $sdkCoreDist) -and (Test-Path $apiClientDist)) { | ||
| return | ||
| } | ||
|
|
||
| & pnpm --dir $RepoRoot --filter @cipherbox/crypto build | ||
| if ($LASTEXITCODE -ne 0) { throw "Failed to build @cipherbox/crypto" } | ||
|
|
||
| & pnpm --dir $RepoRoot --filter @cipherbox/core build | ||
| if ($LASTEXITCODE -ne 0) { throw "Failed to build @cipherbox/core" } | ||
|
|
||
| & pnpm --dir $RepoRoot --filter @cipherbox/api-client build | ||
| if ($LASTEXITCODE -ne 0) { throw "Failed to build @cipherbox/api-client" } | ||
|
|
||
| & pnpm --dir $RepoRoot --filter @cipherbox/sdk-core build | ||
| if ($LASTEXITCODE -ne 0) { throw "Failed to build @cipherbox/sdk-core" } | ||
| } |
There was a problem hiding this comment.
Check all verifier prerequisites before returning early.
This guard only proves two of the four dist outputs you treat as prerequisites are present. A partially-built workspace can skip the fallback and then fail later when the verifier runs. The same guard in tests/desktop-e2e/scripts/test-round-trip.sh needs the same change.
🧱 Proposed fix
function Ensure-VerifierRuntime {
$sdkCoreDist = Join-Path $RepoRoot "packages/sdk-core/dist/index.mjs"
$apiClientDist = Join-Path $RepoRoot "packages/api-client/dist/index.mjs"
+ $coreDist = Join-Path $RepoRoot "packages/core/dist/index.mjs"
+ $cryptoDist = Join-Path $RepoRoot "packages/crypto/dist/index.mjs"
- if ((Test-Path $sdkCoreDist) -and (Test-Path $apiClientDist)) {
+ if ((Test-Path $sdkCoreDist) -and (Test-Path $apiClientDist) -and (Test-Path $coreDist) -and (Test-Path $cryptoDist)) {
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function Ensure-VerifierRuntime { | |
| $sdkCoreDist = Join-Path $RepoRoot "packages/sdk-core/dist/index.mjs" | |
| $apiClientDist = Join-Path $RepoRoot "packages/api-client/dist/index.mjs" | |
| if ((Test-Path $sdkCoreDist) -and (Test-Path $apiClientDist)) { | |
| return | |
| } | |
| & pnpm --dir $RepoRoot --filter @cipherbox/crypto build | |
| if ($LASTEXITCODE -ne 0) { throw "Failed to build @cipherbox/crypto" } | |
| & pnpm --dir $RepoRoot --filter @cipherbox/core build | |
| if ($LASTEXITCODE -ne 0) { throw "Failed to build @cipherbox/core" } | |
| & pnpm --dir $RepoRoot --filter @cipherbox/api-client build | |
| if ($LASTEXITCODE -ne 0) { throw "Failed to build @cipherbox/api-client" } | |
| & pnpm --dir $RepoRoot --filter @cipherbox/sdk-core build | |
| if ($LASTEXITCODE -ne 0) { throw "Failed to build @cipherbox/sdk-core" } | |
| } | |
| function Ensure-VerifierRuntime { | |
| $sdkCoreDist = Join-Path $RepoRoot "packages/sdk-core/dist/index.mjs" | |
| $apiClientDist = Join-Path $RepoRoot "packages/api-client/dist/index.mjs" | |
| $coreDist = Join-Path $RepoRoot "packages/core/dist/index.mjs" | |
| $cryptoDist = Join-Path $RepoRoot "packages/crypto/dist/index.mjs" | |
| if ((Test-Path $sdkCoreDist) -and (Test-Path $apiClientDist) -and (Test-Path $coreDist) -and (Test-Path $cryptoDist)) { | |
| return | |
| } | |
| & pnpm --dir $RepoRoot --filter `@cipherbox/crypto` build | |
| if ($LASTEXITCODE -ne 0) { throw "Failed to build `@cipherbox/crypto`" } | |
| & pnpm --dir $RepoRoot --filter `@cipherbox/core` build | |
| if ($LASTEXITCODE -ne 0) { throw "Failed to build `@cipherbox/core`" } | |
| & pnpm --dir $RepoRoot --filter `@cipherbox/api-client` build | |
| if ($LASTEXITCODE -ne 0) { throw "Failed to build `@cipherbox/api-client`" } | |
| & pnpm --dir $RepoRoot --filter `@cipherbox/sdk-core` build | |
| if ($LASTEXITCODE -ne 0) { throw "Failed to build `@cipherbox/sdk-core`" } | |
| } |
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] 45-45: The cmdlet 'Ensure-VerifierRuntime' uses an unapproved verb.
(PSUseApprovedVerbs)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/desktop-e2e/scripts/test-round-trip.ps1` around lines 45 - 64, The
early-return guard in Ensure-VerifierRuntime only checks $sdkCoreDist and
$apiClientDist but must verify all four build outputs; update
Ensure-VerifierRuntime to define and test paths for the missing dist files
(packages/crypto/dist/index.mjs and packages/core/dist/index.mjs) and change the
if condition to require all four Test-Path results before returning early, and
make the equivalent change in the shell counterpart test-round-trip.sh so both
scripts check the same four prerequisites.
This was referenced Jun 15, 2026
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
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
next_file_publish_sequencehandling in the FUSE desktop upload flowTesting
Changed areas
crates/fusepackages/sdk-core/scripts/verify-filepointer.mjstests/desktop-e2e/scriptsSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests