Skip to content

fix: keep tar archive paths local - #723

Merged
Astro-Han merged 2 commits into
devfrom
codex/fix-windows-tar-path
May 18, 2026
Merged

fix: keep tar archive paths local#723
Astro-Han merged 2 commits into
devfrom
codex/fix-windows-tar-path

Conversation

@Astro-Han

Copy link
Copy Markdown
Owner

Summary

Fix sync-officecli-skills tar invocations so local tarball paths are passed to tar as ./<basename> from the tarball directory instead of as absolute archive paths.

Why

Current dev has required CI green after #717, but windows-advisory / unit-windows-desktop still fails in the OfficeCLI skills sync tests. On Windows, GNU tar treats absolute paths like D:\...\bundle.tar.gz or C:\...\bundle.tar.gz as remote host:file archive syntax, producing errors such as Cannot connect to D: resolve failed.

Related Issue

No linked issue. This is a focused follow-up to the failing windows-advisory run on current dev head d5e6186cca6adca930793872892e26e8cdf67e07.

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

Please check that localTarArchive() is the right boundary: both extraction and the test helper's fixture tarball creation now avoid passing drive-letter archive paths directly to tar.

Risk Notes

Low. This only changes how the maintenance script calls local tar; archive contents and sync behavior are unchanged. Windows impact is the main target. macOS/Linux keep using standard tar with a directory-local archive argument.

How To Verify

Focused red test: localTarArchive test failed before implementation because the export did not exist
Focused tests: cd packages/desktop-electron && bun test scripts/sync-officecli-skills.test.ts -> 24 pass, 0 fail
Desktop typecheck: cd packages/desktop-electron && bun run typecheck -> pass
Desktop CI entry: bun turbo test:ci --filter=@opencode-ai/desktop-electron -> 396 pass, 0 fail
Diff check: git diff --check -> no whitespace errors

Screenshots or Recordings

Not applicable. No visible UI change.

Checklist

  • Human review status is stated above as pending, approved, or not required
  • I linked the related issue, or stated why there is no issue
  • This PR has exactly one type label (bug, enhancement, task, or documentation), at least one primary routing label (app, ui, platform, harness, or ci), and exactly one priority label (P0 to P3), or I requested maintainer labeling
  • I described the review focus and any meaningful risks
  • I listed the relevant verification steps and the key result for each
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
  • I reviewed the final diff for unrelated changes and suspicious dependency changes
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English

@coderabbitai

coderabbitai Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 58 minutes and 27 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6fad4fbb-fadb-47e1-896d-0a3242b2e027

📥 Commits

Reviewing files that changed from the base of the PR and between d5e6186 and d97530b.

📒 Files selected for processing (2)
  • packages/desktop-electron/scripts/sync-officecli-skills.test.ts
  • packages/desktop-electron/scripts/sync-officecli-skills.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-windows-tar-path

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 and usage tips.

@github-actions github-actions Bot added platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions P2 Medium priority labels May 18, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested priority: P2 (includes non-doc, non-test paths outside the low-risk bucket).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@Astro-Han Astro-Han added bug Something isn't working ci Continuous integration / GitHub Actions labels May 18, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a localTarArchive utility to handle tar operations in a cross-platform manner, specifically addressing issues with Windows drive letters by executing commands within the target directory using relative paths. Feedback suggests improving the platform simulation logic in localTarArchive by using path.posix as a fallback instead of the default path module to ensure consistent behavior during cross-platform testing.

Comment thread packages/desktop-electron/scripts/sync-officecli-skills.ts Outdated
@github-actions github-actions Bot removed the ci Continuous integration / GitHub Actions label May 18, 2026
@Astro-Han
Astro-Han merged commit 8c9cdce into dev May 18, 2026
33 checks passed
@Astro-Han
Astro-Han deleted the codex/fix-windows-tar-path branch May 18, 2026 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working P2 Medium priority platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant