Fix deploy silently truncating tarball on dangling symlink#1718
Merged
Conversation
A dangling symlink in the project tree made harper deploy ship a truncated component: tar-fs packages with dereference:true, and when the walker stats a broken symlink target it treats the resulting ENOENT as end-of-stream, finalizing the archive early with no error. Every entry queued after the link in the walk order was silently dropped, while deploy_component reported success. Skip dangling symlinks during packaging so the walk continues past them, and warn the CLI user which links were skipped so the omission is visible instead of silent. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-entry stat Gemini flagged that tar-fs's ignore callback is synchronous (no Promise support), so the previous fix's per-entry lstatSync/statSync would have added blocking I/O to a path that also runs inline on the Harper server's event loop (package_component). Consolidate size + dangling detection into one scanPackageDirectory() async walker; the pack's ignore callback now does a cheap Set lookup against a pre-resolved set. Also fixes a correctness gap the sync version had: the walker now recurses into valid symlinked directories (matching tar-fs's own dereferenced traversal), so a dangling symlink nested inside one is still caught instead of still truncating the archive. The CLI's prepare step now does a single combined scan instead of two separate walks, and threads the result into streamPackagedDirectory to avoid a third. Add a DESIGN.md entry capturing the tar-fs early-finalize mechanism. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request resolves an issue where dangling symlinks silently truncate the deployment tarball. It introduces a pre-walk scan (scanPackageDirectory) to identify broken symlinks and skip them during archiving, while also warning the user. The feedback suggests improving stream lifecycle and resource management in streamPackagedDirectory by handling early aborts and preventing resource leaks if the gzip stream is closed prematurely.
Contributor
|
Reviewed; no blockers found. |
Addresses Gemini PR review feedback: the manual packStream.pipe(gzip) didn't propagate destruction in either direction. If the client aborted while the dangling-symlink prescan was still resolving, we'd still kick off a pointless directory walk; if it aborted after the walk started, packStream kept running with nowhere for its output to go. pipeline() destroys both streams on either side erroring or closing early, and the destroyed-gzip check skips starting the walk at all for the first case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Patch cherry-pick: conflictCherry-pick onto The conflict markers are committed on branch |
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 8, 2026
Fix deploy silently truncating tarball on dangling symlink
kriszyp
added a commit
that referenced
this pull request
Jul 9, 2026
cherry-pick: Fix deploy silently truncating tarball on dangling symlink (#1718)
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
harper deploy(and the server-sidepackage_componentoperation) could silently ship a truncated component when the project tree contained a dangling symlink — reported by @dillardm89.Root cause: packaging uses
tar-fs.pack(dir, { dereference: true }). tar-fs's walkerstats every entry when dereferencing; a dangling symlink's target throwsENOENT, and tar-fs'sstatAllloop treats anyENOENTon a walk-discovered entry as end-of-stream — it callspack.finalize()immediately, silently dropping every entry still queued after the link (BFS order). No error is ever emitted, so the existingpackStream.on('error', ...)guard never fires, anddeploy_componentreports success on the truncated archive. Confirmed by direct reproduction against the installedtar-fs3.1.2.Fix:
scanPackageDirectory()walks the tree once (async) to build a set of dangling symlinks;streamPackagedDirectory'star.pack({ ignore })skips them via a synchronousSetlookup, so tar-fs's walk continues past the broken link instead of finalizing early. The scan recurses into valid symlinked directories the same way tar-fs's dereferenced walk does, so a dangling link nested inside one is still caught — it would otherwise still trip the same early-finalize. The CLI warns to stderr listing any skipped links so the omission is visible instead of silent.Purpose
Prevent a dangling symlink anywhere in a deployed project from silently dropping every file after it in the tarball, while giving the CLI user visibility into any links that had to be skipped.
Where to look
components/packageComponent.ts— the core fix (scanPackageDirectory, theignorecallback change).DESIGN.md— new section documenting the tar-fs early-finalize mechanism for future readers.Open item for the human reviewer
The server-side
package_component/deploy_componentpath has no equivalent warning surface — only the CLI prints the skipped-links message. The truncation bug itself is fixed on both paths, but a server-initiated deploy still silently omits the dangling link's (nonexistent) content with no signal. The cross-model review recommended this as the strongest case for a follow-up: post-extract validation that declared entry points (jsResource/graphqlSchema) survived extraction, which would catch a truncation from any future cause, not just this one. Deliberately scoped out of this PR as a larger, separate change — filed as #1719.Docs
No documentation-repo change — this is a bug fix (packaging correctness) plus an informational CLI warning, not a new flag, config option, or documented contract.
Generated by Claude (Opus 4.8 / Sonnet 5), reviewed via cross-model review (Gemini + Harper-domain adjudication).