Skip to content

Keep watched archives searchable when recursion is disabled - #778

Merged
golift-bot merged 1 commit into
bump/xtractr-readdir-emptyfrom
fix/disable-recursion-exclude
Sep 21, 2026
Merged

golift-bot merged 1 commit into
bump/xtractr-readdir-emptyfrom
fix/disable-recursion-exclude

Conversation

@davidnewhall

Copy link
Copy Markdown
Collaborator

Summary

  • Stop passing every archive suffix as ExcludeSuffix for a watched archive with disable_recursion.
  • xtractr now applies that list when the search path is the archive itself, so the old list made TestFolderDisableRecursion report "no compressed files found". Nested archives are still skipped by DisableRecursion.

Stacked on #777.

Test plan

  • go test -run 'TestFolderDisableRecursion|TestFolderExcludeSuffixes' ./pkg/unpackerr/
  • Integration TestFolderDisableRecursion against this branch

Made with Cursor

xtractr now honors ExcludeSuffix for a path that is the archive, so listing every suffix made disable_recursion skip the file instead of only its nested archives.

Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The filtering change matches xtractr’s direct-archive behavior and is covered by focused tests.

Review effort: Lite
Findings: None

What changed in this PR

Updates watched-folder archive filtering so root archives remain searchable when recursion is disabled.

Changes:

  • Simplifies archive suffix exclusions to only honor extract_isos.
  • Updates recursion tests for direct archive discovery and extraction.
File Description
pkg/​unpackerr/​folder.go Adjusts suffix filtering for watched archives.
pkg/​unpackerr/​folder_recursion_test.go Updates coverage for recursion and archive search behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@qwen-pr-bot qwen-pr-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approve

Reviewed head 6efbe62 against base 2ff4c37 (the xtractr bump branch). One change in practice: folderExcludeSuffixes loses its "exclude every archive suffix for a watched archive file" hack and keeps only the .iso exclusion when extract_isos=false; both call sites (extractTrackedItem, folderArchiveCount) and the recursion tests are updated to match.

Why the old code was broken

The Feb 2026 commit 8536c59 added the all-suffix exclusion to make xtractr skip nested archives, but xtractr applies ExcludeSuffix to the search path itself (findDirectArchive in xtractr find.go: "excluded suffix, even when the archive is passed in directly"). So a watched foo.zip with disable_recursion=true was unfindable by its own exclusion list. I reproduced it at base: the pre-existing TestFolderDisableRecursionHonored fails there with "no compressed files found", and the old exclude list for a watched zip contained all 40 supported extensions, so FindCompressedFiles returned 0 archives. The same list fed folderArchiveCount, so such archives also reported 0 archives and got dropped by skip_empty. Two real regressions fixed here.

Why the fix is safe

The current xtractr (v0.6.2-0.20260921...) honors recursion natively: decompressFiles skips the follow-up scan entirely when DisableRecursion is set, and decompressFolders propagates the flag into per-folder sub-responses. The follow-up scan was the only thing the suffix hack was ever protecting against, so nothing is lost by dropping it. Watched directories were unaffected either way, since the old code returned early for non-archive paths.

Executed validation

At head 6efbe62:

  • go build ./... and go vet ./pkg/unpackerr/ — clean
  • go test ./pkg/unpackerr/ -run 'TestFolder' -count=1 — all pass, including TestFolderDisableRecursionHonored (nested zip stays unextracted, sibling content extracts) and TestFolderDisableRecursionFalseExtractsNested (recursion-on still works)
  • go test ./... -count=1 — all seven testable packages pass

At base 2ff4c37 (separate worktree, scratch repro removed afterwards): the recursion test fails as described above, confirming the defect this PR fixes exists at base and is gone at head.

The flipped assertion in TestFolderExcludeSuffixesArchiveStaysSearchableWhenDisableRecursion, checking that FindCompressedFiles actually returns the watched archive, is the right regression test for this class of bug. No docs in this repo mention the old behavior.

@golift-bot
golift-bot merged commit e11a033 into bump/xtractr-readdir-empty Sep 21, 2026
16 checks passed
@golift-bot
golift-bot deleted the fix/disable-recursion-exclude branch September 21, 2026 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants