fix: resolve gateway binary path, pass --config flag, and clarify emp…#1337
Conversation
|
|
sipeed-bot
bot
added
type: bug
snac21
force-pushed
the
fix/gateway-binary-path-and-config
branch
from
64e6118 to
9c3ef70
Compare
web/backend/api/gateway.go
Outdated
| cmd := exec.Command(execPath, "gateway") | ||
| args := []string{"gateway"} | ||
| if h.configPath != "" { | ||
| args = append(args, "--config", h.configPath) |
There was a problem hiding this comment.
picoclaw gateway does not support a --config flag today, so this change will make launcher-driven gateway startup fail immediately.
The subcommand only defines --debug and --no-truncate, and still uses cobra.NoArgs, so passing --config <path> causes the child process to exit with unknown flag: --config.
Since the launcher always has a non-empty config path, this affects both auto-start and manual start. If the goal is to make the gateway use the launcher’s config file, a safer approach would be to set PICOCLAW_CONFIG in the child process environment before cmd.Start(), because the CLI already resolves its config path from that environment variable.
There was a problem hiding this comment.
Updated it as requested.Could you please review it again?
web/backend/api/gateway.go
Outdated
| return candidate | ||
| } | ||
|
|
||
| // 3. Relative paths — covers running from web/backend/dist/ |
There was a problem hiding this comment.
The relative-path fallback here looks unnecessary. In our actual build layout, both picoclaw-launcher and picoclaw are emitted into the same build/ directory, so the previous “same directory as the current executable” check should already cover the production case.
For typical development flows, this ../../build / ../build heuristic is also fairly brittle because os.Executable() often points to a temporary Go build location rather than web/backend/dist/.
Given that this PR already adds PICOCLAW_BINARY as an explicit override, I think that plus same-directory lookup and the final $PATH fallback is the robust part; this extra relative-path probing seems speculative and may not be worth keeping unless there is a concrete packaging layout that depends on it.
snac21
force-pushed
the
fix/gateway-binary-path-and-config
branch
from
9c3ef70 to
be83866
Compare
|
@snac21 Nice work tackling three issues in one PR. Using PICOCLAW_BINARY as an explicit override and removing the fragile relative path probing is much more robust. The clear error message for empty model config is also a good UX improvement. We have a PicoClaw Dev Group on Discord for contributors. Want to join? Send an email to |
4 participants
This PR addresses the 3 issues reported regarding the gateway startup process, incorporating the latest feedback:
findPicoclawBinary()now checks thePICOCLAW_BINARYenvironment variable first as an explicit override. As suggested, the fragile relative path probing (../../build/picoclaw) was removed. It now falls back to the same directory as the executable (which naturally covers the production build layout) before relying on$PATH.PICOCLAW_CONFIGenvironment variable in the child process beforecmd.Start(). This seamlessly integrates with the existing CLI config resolution logic.pkg/providers/factory.go. Instead of panicking with a confusing "no API key configured for model: " error when an empty model reaches the default switch case, it now checks if both the provider and model are empty and returns a clear"no model configured: agents.defaults.model is empty"error early.All tests have been updated and pass successfully.
Fixes #1290