-
Notifications
You must be signed in to change notification settings - Fork 17
Fix varlock path compatibility for debug/startup/subagents #203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -252,12 +252,19 @@ echo "=== Installing varlock ===" | |
| # varlock must be available to the agent user (start.sh adds ~/.varlock/bin to PATH). | ||
| # Install as agent user so it lands in the right home directory. | ||
| AGENT_VARLOCK="$BAUDBOT_HOME/.varlock/bin/varlock" | ||
| if [ -x "$AGENT_VARLOCK" ]; then | ||
| AGENT_VARLOCK_CONFIG_BIN="$BAUDBOT_HOME/.config/varlock/bin/varlock" | ||
| if [ -x "$AGENT_VARLOCK" ] || [ -x "$AGENT_VARLOCK_CONFIG_BIN" ]; then | ||
| echo "varlock already installed for baudbot_agent, skipping" | ||
| else | ||
| sudo -u baudbot_agent bash -c 'curl -sSfL https://varlock.dev/install.sh | sh -s' | ||
| fi | ||
|
|
||
| # Newer varlock installers place the binary under ~/.config/varlock/bin. | ||
| # Keep a compatibility link at ~/.varlock/bin/varlock for existing runtime scripts. | ||
| if [ -x "$AGENT_VARLOCK_CONFIG_BIN" ]; then | ||
| sudo -u baudbot_agent bash -c "mkdir -p '$BAUDBOT_HOME/.varlock/bin' && ln -sf '$AGENT_VARLOCK_CONFIG_BIN' '$AGENT_VARLOCK'" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If Consider guarding with a check that skips the symlink creation if the legacy path already contains a functional binary that is not a symlink: if [ -x "$AGENT_VARLOCK_CONFIG_BIN" ] && [ ! -x "$AGENT_VARLOCK" ]; then
sudo -u baudbot_agent bash -c "mkdir -p '$BAUDBOT_HOME/.varlock/bin' && ln -sf '$AGENT_VARLOCK_CONFIG_BIN' '$AGENT_VARLOCK'"
fiThis creates the compatibility link only when the legacy path is absent, avoiding accidental replacement of an existing real binary. Prompt To Fix With AIThis is a comment left during a code review.
Path: setup.sh
Line: 264-265
Comment:
**`ln -sf` silently overwrites a real binary when both install locations coexist**
If `~/.varlock/bin/varlock` already exists as a real (non-symlink) binary — e.g., a legacy install — and `~/.config/varlock/bin/varlock` also exists, `ln -sf` will silently delete the real binary and replace it with a symlink. While the symlink points to an equally valid binary, this is a destructive and potentially surprising operation on re-runs of `setup.sh`.
Consider guarding with a check that skips the symlink creation if the legacy path already contains a functional binary that is not a symlink:
```bash
if [ -x "$AGENT_VARLOCK_CONFIG_BIN" ] && [ ! -x "$AGENT_VARLOCK" ]; then
sudo -u baudbot_agent bash -c "mkdir -p '$BAUDBOT_HOME/.varlock/bin' && ln -sf '$AGENT_VARLOCK_CONFIG_BIN' '$AGENT_VARLOCK'"
fi
```
This creates the compatibility link only when the legacy path is absent, avoiding accidental replacement of an existing real binary.
How can I resolve this? If you propose a fix, please make it concise.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Fixed in d666663.
Responded by baudbot-dev-agent using openai/gpt-5. |
||
| fi | ||
|
|
||
| echo "=== Publishing initial git-free /opt release ===" | ||
| # Build an immutable release snapshot from the local source checkout, then deploy | ||
| # from /opt/baudbot/releases/<sha>. This keeps live operations decoupled from | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
elifsilences telemetry warning for new config path when legacy config also matchesBecause the new config-path check is behind an
elif, if~/.varlock/config.jsonexists and containsanonymousId, the check for~/.config/varlock/config.jsonis never reached — even if that file also containsanonymousId. On systems where both config files are present (e.g., migrated installs), the new-path telemetry issue would go undetected.Use two independent
ifblocks so both paths are always checked:Prompt To Fix With AI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Fixed in d666663.
I changed the telemetry checks in
bin/doctor.shto use two independentifblocks, so both legacy (~/.varlock/config.json) and current (~/.config/varlock/config.json) paths are checked even when both exist.Responded by baudbot-dev-agent using openai/gpt-5.