feat: support native artifact downloads on Windows - #346
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughWindows now supports native artifact downloads. The change adds Windows path validation, native directory-handle protection, platform-specific integration, tests, runtime bindings, and documentation updates. Linux behavior remains descriptor-based. macOS and BSD remain unsupported. ChangesWindows artifact downloads
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant ArtifactDownload
participant ArtifactTools
participant WindowsDestination
participant WindowsFilesystem
ArtifactDownload->>ArtifactTools: request artifact download
ArtifactTools->>WindowsDestination: prepare validated destination
WindowsDestination->>WindowsFilesystem: pin directory handles and reject reparse points
WindowsFilesystem-->>WindowsDestination: validated destination handle
WindowsDestination-->>ArtifactTools: destination handle and anchor path
ArtifactTools-->>ArtifactDownload: write and publish artifact
Suggested reviewers: Merge Risk: ⚪ Minimal · up to Windows artifact downloads add validated paths and protected destination handling without an identified merge-blocking regression. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 5.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 4 files. (5 skipped: 5 unsupported.)
✨ 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. A rabbit checks each path with care Comment |
Greptile SummaryThis change enables native artifact downloads on Windows, adds Windows-specific safe-path validation, and pins workspace and destination directories before using the existing streaming, integrity verification, stale-partial cleanup, and hard-link publication flow. Confidence Score: 5/5Safe to merge: no blocking or non-blocking defects were established. The focused artifact checks passed, including safe-path rejection, real filesystem publication, traversal rejection, type checking, and the artifact download regression test. Files Needing Attention: None.
What T-Rex did
Reviews (1): Last reviewed commit: "feat: support native artifact downloads ..." | Re-trigger Greptile |
download_artifact is currently Linux-only because secure traversal relies on descriptor-anchored filesystem operations. This PR adds Windows support while keeping the current Linux implementation unchanged. On Windows, the workspace root and each destination directory are pinned with native CreateFileW handles via Koffi; reparse points are rejected and delete sharing is omitted so path components cannot be renamed or replaced during transfer. Existing streaming, size/hash verification, same-directory partial files, and hard-link no-overwrite publication remain shared. Windows-specific validation rejects drive-relative or absolute paths, UNC and device namespaces, ADS, reserved device names, invalid characters, and trailing dot or space segments. Koffi is lazy-loaded only on Windows and is included in the pnpm build-script allowlist. This is intentionally narrower than #103 and based on current main: it carries forward the native Windows handle-pinning approach without refactoring the Linux path or adding macOS support. Verified on Windows with pnpm typecheck, the full test suite (131 passed, 6 skipped, 0 failed), and pnpm build. pnpm test:package-install currently fails on Windows with the same devspace.cmd path-not-found error on an untouched current upstream/main checkout, so that failure is pre-existing and not introduced by this branch.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation