Skip to content

test(scripts): skip POSIX helper permission checks on Windows - #21766

Closed
UtkarshUsername wants to merge 1 commit into
pingdotgg:mainfrom
UtkarshUsername:fix/linux-helper-test-windows
Closed

UtkarshUsername wants to merge 1 commit into
pingdotgg:mainfrom
UtkarshUsername:fix/linux-helper-test-windows

Conversation

@UtkarshUsername

Copy link
Copy Markdown
Contributor

Problem

The Linux capture-helper staging test fails on Windows because it expects POSIX executable mode 0755, while Windows reports 0666.

Change

Only assert POSIX permission bits on non-Windows hosts. Windows still verifies staged helper contents, and the Hyprland protocol files remain checked. No production behavior changes.

Scope and approval

This is a small, focused fix for an obvious platform-specific test defect, qualifying for the exception without a prior issue or discussion.

Verification

On Windows x64:

  • Before: the helper staging test failed with expected 0755, received 0666.
  • After: vp test run lib/cli-external-packages.test.ts build-desktop-artifact.test.ts passed all 98 tests in 7.61s. Removed inherited ELECTRON_RUN_AS_NODE only from the test shell to avoid the separate existing environment-sensitive native-probe assertion.
  • vp lint scripts/build-desktop-artifact.test.ts passed.
  • vp run --filter @t3tools/scripts typecheck passed.
  • git diff --check passed.

Linux/macOS execution was not available locally; their permission assertion is preserved.

Model: gpt-6.1-sol. Harness: Codex in T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 19, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at ef41d8c

Macroscope's review found this PR approvable — This is a narrowly scoped, test-only Windows compatibility adjustment in an ignored path; it preserves the substantive staging checks and does not alter product behavior, defaults, or static-analysis policy.

Notes:

  • No code objects were reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d1c666db-f06b-42b6-8fe4-84d07b2efa48
📥 Commits

Reviewing files that changed from the base of the PR and between 7e7e6d8 and ef41d8c.

📒 Files selected for processing: 1
  • scripts/build-desktop-artifact.test.ts

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

📝 Walkthrough

The Linux capture-helper staging test now checks executable permission bits only on non-Windows hosts. Windows still runs the staging test without the POSIX mode assertion.

Changes: Capture-helper staging test

Layer / File(s) Summary
Platform-specific permission assertion
scripts/build-desktop-artifact.test.ts
The test reads the host platform and checks mode 0o755 only when the platform is not win32.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Suggested reviewers: bil0000

Merge Risk: ⚪ Minimal · up to ef41d

The test continues to verify staged helper contents on Windows while checking POSIX permissions on applicable hosts. No actionable merge-blocking risk remains.

### Architecture Summary

Architecture risk: 🔵 Low · up to ef41d

The change affects 1 system.

Changed systems: scripts

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — scripts (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in scripts/build-desktop-artifact.test.ts: The capture-helper staging test now obtains the host platform for use in platform-specific assertions.
  • observed — Modified behavior in scripts/build-desktop-artifact.test.ts: The test now checks mode 0o755 only when the host platform is not win32; the previous unconditional POSIX permission assertion is removed.
Pre-merge checks: Passed 4
Check name Status Explanation
Description check Passed The description includes the problem, change, scope justification, platform-specific verification results, limitations, and agent details. It matches the required template and provides focused test ev…
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.
Title check Passed The title clearly and concisely describes the main change: skipping POSIX helper permission checks on Windows.
Full details: Description check

Explanation

The description includes the problem, change, scope justification, platform-specific verification results, limitations, and agent details. It matches the required template and provides focused test evidence.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@UtkarshUsername UtkarshUsername changed the title fix(scripts): skip POSIX helper permission checks on Windows test(scripts): skip POSIX helper permission checks on Windows Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant