Skip to content

PE-9205: A manifest leaves out hidden folders and loses no file to a path collision - #2265

Merged
vilenarios merged 1 commit into
devfrom
fix-manifest-hidden-and-collisions
Oct 14, 2026
Merged

vilenarios merged 1 commit into
devfrom
fix-manifest-hidden-and-collisions

Conversation

@vilenarios

@vilenarios vilenarios commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Two live bugs in ManifestDataBuilder, which every manifest goes through, from Create Manifest and from AutoDeploy after an upload. Both were reproduced with a test before anything changed.

Hidden folders were published

The builder filtered each file by its own hidden flag. Hiding a folder marks that folder alone, never its contents (hide_bloc.dart:72), and the folder tree keeps hidden folders (foldersInFolder has no isHidden predicate). So a hidden folder's visible files went out at their real paths, permanently, while the manifest form told the user hidden files are left out.

The builder now prunes hidden folders however deep. FolderNode.getRecursiveFiles is unchanged, because the pending-files check also uses it.

Path collisions dropped files

Spaces become underscores, so my file.txt and my_file.txt arrive at the same path, and the paths map kept one of them with no error.

Gateways decode a request's path before matching it against a manifest's keys (ar-io-node src/middleware/arns.ts:46, compared exactly at src/lib/encoding.ts:402), so a raw path with its spaces is a valid key. On a collision every file takes its raw path, which leaves the underscored path with the file whose name it already was. A file that collides with nobody keeps the path it has always had, so no existing URL moves on republish. Two files at exactly the same path are told apart by id, the earliest created keeping the plain path: dup.txt and dup~<8 chars of id>.txt.

Keys are deliberately not percent-encoded. An earlier plan of mine said to encode them; checking the gateway showed that would break every key containing #, ?, % or a space.

Three smaller defects in the same function

  • A hidden index.html became the index while being left out of the paths, so the manifest's root served nothing. The index is now chosen from the files being published.
  • A colliding index pointed at the other file. The index now uses the path its own file was given.
  • A folder with nothing to publish threw an unexplained StateError from .first. It now throws ManifestHasNoFilesException, through the same ManifestCreationException failure path as before. Pruning hidden folders makes this case more reachable, which is why it is handled here.

The form's line now reads "Hidden files and folders are not added to the manifest," which is true.

Verification

  • Ten new tests, plus the existing five unchanged: 15/15. With the two other test files that touch manifests, 47/47.

  • Each rule was broken on purpose and the file rerun. All eight mutants compiled, and each was caught by the test written for it:

    Mutant Caught by
    Hidden folders not pruned everything inside a hidden folder; nothing left to publish
    Index picked from hidden files too never makes a hidden index.html the index
    Collisions not handled both files kept, in either order; raw paths when neither owns the underscored one; index follows its file
    Underscore convention dropped a name that collides with nothing keeps the underscored path
    Exact duplicates not separated two files at the same path
    Empty folder not refused nothing left to publish
    Hidden file flag ignored hidden file in a visible folder; hidden index; nothing left
    Latest created keeps the path two files at the same path, earliest first
  • flutter analyze across the project: no issues.

  • dart format on the edited files only: no changes.

Not in this PR

What the manifest flow shows when a folder has nothing to publish. It fails with the existing generic failure, which is no worse than today's behaviour for an empty folder; disabling the button with a reason is a separate change.

🤖 Generated with Claude Code

https://claude.ai/code/session_017NRgRqqrM4sg2EeqbGJzff

Summary by CodeRabbit

  • New Features
    • Manifests now include files from visible subfolders, while excluding hidden files, files in hidden folders, and existing manifests.
    • When file paths conflict, published files receive distinct paths. The index prefers a publishable root-level index.html, otherwise it uses the first publishable file.
  • Bug Fixes
    • Creating a manifest with no publishable files now reports an error.
    • The folder-selection notice now clarifies that hidden files and folders are excluded.

… collision PE-9205

Two live bugs in ManifestDataBuilder, both reproduced with a test before
the fix.

Hidden folders were published. The builder filtered each file by its own
hidden flag, but hiding a folder marks that folder alone, so a hidden
folder's visible files went out at their real paths while the manifest
form said hidden files are excluded. The builder now prunes hidden
folders however deep, without changing FolderNode.getRecursiveFiles,
which the pending-files check also uses.

Path collisions dropped files. Spaces become underscores, so
"my file.txt" and "my_file.txt" share a path, and the paths map kept
one of them. Gateways decode a request's path before matching it against
a manifest's keys (ar-io-node src/middleware/arns.ts:46), so a raw path
is a valid key: on a collision every file takes its raw path, which
leaves the underscored path with the file whose name it already was. A
file that collides with nobody keeps the path it has always had, so no
existing URL moves. Two files at exactly the same path are told apart
by id, the earliest created keeping the plain path.

Three smaller defects in the same function:

- A hidden index.html became the index while being left out of the
  paths, so the manifest's root served nothing. The index is now chosen
  from the files being published.
- When the index collided, it pointed at the other file's path.
- A folder with nothing to publish threw an unexplained StateError. It
  now throws ManifestHasNoFilesException, through the same failure path.

The form's claim now says hidden files and folders are left out, which
is true.

Each rule was broken on purpose and the suite rerun: all eight mutants
compiled and each failed the test written for it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017NRgRqqrM4sg2EeqbGJzff
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: ardriveapp/ardrive-web/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 281208a2-fbe1-4234-86e9-6ac59e6a2e8d
📥 Commits

Reviewing files that changed from the base of the PR and between bd5de1b and e8cefe5.

📒 Files selected for processing: 3
  • lib/components/create_manifest_form.dart
  • lib/entities/manifest_data.dart
  • test/entities/manifest_data_test.dart

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The manifest builder now excludes hidden files, files under hidden folders, and existing manifests. It rejects builds with no publishable files, selects a publishable index, and assigns paths that resolve normalized and duplicate-path collisions.

Changes: Manifest building

Layer / File(s) Summary
Select publishable files
lib/entities/manifest_data.dart, test/entities/manifest_data_test.dart, lib/components/create_manifest_form.dart
The builder collects files from visible folders, excludes hidden files and existing manifests, and throws ManifestHasNoFilesException if no publishable files remain. It selects a publishable root index.html when available. Tests cover filtering, index selection, and empty results. The folder-selection notice now says hidden files and folders are excluded.
Assign manifest paths
lib/entities/manifest_data.dart, test/entities/manifest_data_test.dart
The builder prepares paths relative to the selected folder. It resolves normalized-path and duplicate-path collisions, and uses the assigned path for the index. Tests cover path collisions and index paths.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to e8cef

The change excludes hidden folders from the manifest and keeps files that collide on a path. No actionable merge-blocking risk was found in the supplied context.

Pre-merge checks: Passed 5
Passed Passed checks: 5 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes both primary changes: excluding hidden folders and preserving files during path collisions.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • 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

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

@github-actions

Copy link
Copy Markdown

Visit the preview URL for this PR (updated for commit e8cefe5):

https://ardrive-web--pr2265-fix-manifest-hidden-w9slynkp.web.app

(expires Wed, 21 Oct 2026 05:16:03 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: a224ebaee2f0939e7665e7630e7d3d6cd7d0f8b0

@vilenarios
vilenarios merged commit 60f2260 into dev Oct 14, 2026
7 checks passed
@vilenarios
vilenarios deleted the fix-manifest-hidden-and-collisions branch October 14, 2026 12:39
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