node_runner: prefer existing local ZeuZ Node over GitHub download#667
node_runner: prefer existing local ZeuZ Node over GitHub download#667
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 175946ac83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Apps/node_runner/main.go
Outdated
| if !strings.HasPrefix(name, "ZeuZ_Node-") { | ||
| continue | ||
| } | ||
| if containsNodeCLI(name) { | ||
| return name, nil |
There was a problem hiding this comment.
This fallback loop returns the first ZeuZ_Node-* directory with node_cli.py, even when the user explicitly requested a different --branch or tagged version, so setupZeuzNode() can skip download and execute an unrelated stale checkout. The regression appears whenever expectedDir is absent but any other ZeuZ_Node folder exists, which silently changes runtime behavior and can invalidate runs that rely on a specific revision.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6120c27fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sort.Strings(candidates) | ||
| return candidates[0], nil |
There was a problem hiding this comment.
After collecting any ZeuZ_Node-* folder with node_cli.py, this code always returns the lexicographically first candidate, even when expectedDir (derived from the requested --branch or tagged version) is missing. In that scenario setupZeuzNode() silently skips download and runs an unrelated local revision, which can execute stale code and produce results that do not match the requested build.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Updated
findExistingZeuzNodeDir()so it no longer falls back to an unrelated localZeuZ_Node-*directory when a specific revision was requested via--branchor a taggedversion; it now only uses arbitrary local fallback for default/dev resolution. This ensuressetupZeuzNode()will proceed to download the requested revision when the exact local directory is missing. Apps/node_runner/main.goL157-L191 - Committed the fix on branch
workas commitfe773ca. - Created a follow-up PR using the
make_prtool with title: “Follow-up: honor requested ZeuZ Node revision before local fallback”.
Testing
- ✅
gofmt -w Apps/node_runner/main.go - ✅
cd Apps/node_runner && go test ./... ⚠️ curl -I https://github.com -m 10 | head -n 1(network/proxy in this environment returned HTTP 403 on CONNECT)
1 participant
Motivation
Description
ZeuZ_Node-devwhen no--branchor taggedversionis provided by updatinggetZeuZNodeDir().containsNodeCLI()to detect a valid node copy by checking fornode_cli.pyinside a directory.findExistingZeuzNodeDir()to search the current directory for an existingZeuZ_Node-*folder that containsnode_cli.pyand return it when found.setupZeuzNode()to return the resolved directory (string, error) and to skip download/extract when a valid local copy is found, and updatedmain()to use the resolved directory before callingos.Chdir.Testing
gofmt -w Apps/node_runner/main.go, which completed successfully.cd Apps/node_runner && go test ./..., which completed successfully (packages reported with no test files).Codex Task