Skip to content

External Agent: Add Windows-compatible agent name derivation#729

Merged
nodo merged 3 commits intoentireio:mainfrom
keyu98:feature/windows-support-external-agent
Mar 25, 2026
Merged

External Agent: Add Windows-compatible agent name derivation#729
nodo merged 3 commits intoentireio:mainfrom
keyu98:feature/windows-support-external-agent

Conversation

@keyu98
Copy link
Contributor

@keyu98 keyu98 commented Mar 19, 2026

Summary

External agent discovery currently fails to correctly derive agent names on Windows because:

  1. Windows executables have file extensions (.exe, .bat, .cmd) that get included in the agent name.
  2. The Unix executable-bit check (mode & 0o111) rejects all binaries on Windows, since Windows doesn't set POSIX execute permission bits.

Changes

  • Strip Windows executable extensions before deriving the agent name via a new stripExeExt() helper. This removes .exe, .bat, and .cmd suffixes so that entire-agent-foo.exe correctly resolves to agent name foo. On Unix this is a no-op.
  • Skip the executable-bit check on Windows (runtime.GOOS != "windows"), since Windows filesystems do not expose POSIX permission bits.

Files changed

  • cmd/entire/cli/agent/external/discovery.go

@keyu98 keyu98 requested a review from a team as a code owner March 19, 2026 09:25
@keyu98 keyu98 changed the title feat: Add Windows-compatible agent name derivation External Agent: Add Windows-compatible agent name derivation Mar 19, 2026
Copy link
Contributor

@nodo nodo left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution @keyu98 ❤️ I have added a suggestion to simplify stripExeExt, also do you mind adding tests for it? The idea is great, just a couple of minor points.

@keyu98
Copy link
Contributor Author

keyu98 commented Mar 24, 2026

Thank you for the contribution @keyu98 ❤️ I have added a suggestion to simplify stripExeExt, also do you mind adding tests for it? The idea is great, just a couple of minor points.

Thanks for the review! I've simplified stripExeExt as suggested and added tests for it in the latest commit.

Copy link
Contributor

@nodo nodo left a comment

Choose a reason for hiding this comment

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

LGTM, please have a look at the failing test, then I am happy to approve.

@keyu98 keyu98 force-pushed the feature/windows-support-external-agent branch from c5c78b8 to fa77853 Compare March 25, 2026 06:19
@keyu98 keyu98 requested a review from nodo March 25, 2026 06:23
@nodo
Copy link
Contributor

nodo commented Mar 25, 2026

Thank you @keyu98 !

@nodo nodo merged commit d3f4a4a into entireio:main Mar 25, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants