Skip to content

Fix Windows worker executable resolution - #266

Draft
bobaikato wants to merge 1 commit into
safal207:mainfrom
bobaikato:bobai/pythia-windows-worker-path
Draft

Fix Windows worker executable resolution#266
bobaikato wants to merge 1 commit into
safal207:mainfrom
bobaikato:bobai/pythia-windows-worker-path

Conversation

@bobaikato

@bobaikato bobaikato commented Sep 1, 2026

Copy link
Copy Markdown

Finding

The released Ota v1.6.27 matrix built the Rust worker successfully on Windows, but the Elixir test searched for:

workers/solver_port/target/release/solver_port

Cargo emits solver_port.exe on Windows, so the setup callback invalidated the worker tests despite a successful build.

Original evidence:
https://github.com/bobaikato/pythiaLabs/actions/runs/33517820099/job/99894708547

Fix

Select solver_port.exe only when :os.type() reports :win32; retain solver_port everywhere else.

Validation

  • final Windows artifact: verify-worker-build=0, verify-worker-test=0, and verify-elixir-test=0
  • exact combined repair matrix: https://github.com/bobaikato/pythiaLabs/actions/runs/33540883165
  • diff and source shape checked locally; Mix is unavailable on the authoring host, so hosted Windows execution is the decisive proof

Existing Elixir-format and Prettier deviations remain separate and unchanged.

Scope

Only test/port_worker_test.exs changes. There are no formatting, refactoring, dependency, Ota-contract, or workflow changes.

This draft is separate from Ota adoption PR #264 and implies no merge approval or endorsement.

Summary by CodeRabbit

  • Tests
    • Updated port worker tests to use the correct worker binary name on Windows and other platforms.

safal207 commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Exact-head review: e62820c8f3a22ec6f26b027805e2a0ff1a73f509.

The change is limited to selecting solver_port.exe on win32 while preserving the existing Unix binary name elsewhere. This is the correct refactor point for the observed Windows artifact lookup failure; no unrelated scope expansion found.

Advisory status: HOLD, not PASS. The code boundary is acceptable, but the current CI/Security runs are action_required and have not executed on this head. Merge or ready-for-review should wait for exact-head execution evidence.

safal207 commented Sep 2, 2026

Copy link
Copy Markdown
Owner

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 602504d9-67a4-4a5e-8dc4-e8b1fe29d5ff

📥 Commits

Reviewing files that changed from the base of the PR and between 17df877 and e62820c.

📒 Files selected for processing (1)
  • test/port_worker_test.exs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The worker test now selects a platform-specific executable name before building the worker binary path.

Changes

Worker binary path

Layer / File(s) Summary
Platform-specific worker selection
test/port_worker_test.exs
The test uses solver_port.exe on Windows and solver_port on other platforms.

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

Merge Risk: ⚪ Minimal · up to e6282

The change makes the Windows test locate the executable Cargo already builds while preserving non-Windows behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the defect, fix, scope, and validation results, but it omits several required template sections. It does not provide the exact 40-character PR head SHA, validation command, re… Add all required template sections. Include the exact 40-character PR head SHA, the validation command, confirmation that validation ran after the latest head change, exact-head evidence, Lotus judgment responses and note, compatibility and…
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the primary change: fixing Windows worker executable resolution.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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.
Full details: Description check

Explanation

The description explains the defect, fix, scope, and validation results, but it omits several required template sections. It does not provide the exact 40-character PR head SHA, validation command, required validation checkboxes, Lotus judgment check, compatibility and authority statement, or complete evidence details. The objectives also state that current CI and security runs are action_required and have not executed on the reviewed commit.

Resolution

Add all required template sections. Include the exact 40-character PR head SHA, the validation command, confirmation that validation ran after the latest head change, exact-head evidence, Lotus judgment responses and note, compatibility and authority impact, and evidence that current CI and security checks executed successfully on the reviewed commit.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants