fix(runtime): make helper paths container-friendly#80
Conversation
Signed-off-by: kesslerio <martin@kessler.io>
Signed-off-by: kesslerio <martin@kessler.io>
chrysb
left a comment
There was a problem hiding this comment.
Left two inline review notes on the runtime path/cron behavior.
| }); | ||
| const gitShimTemplatePath = path.join(__dirname, "..", "lib", "scripts", "git"); | ||
| const gitShimDest = "/usr/local/bin/git"; | ||
| const gitShimDest = resolveGitShimPath(); |
There was a problem hiding this comment.
[P2] Make the custom shim discoverable by OpenClaw. If ALPHACLAW_GIT_SHIM_PATH points outside PATH (for example /state/bin/git), startup writes the shim successfully, but the managed OpenClaw process still inherits PATH unchanged, so plain git calls resolve to the real binary and bypass the askpass shim. Either prepend path.dirname(gitShimDest) to the gateway/onboarding command env when this override is set, or document/enforce that the configured directory must already be first on PATH; otherwise the new env var can look configured while auth still fails.
| kSystemCronConfigPath, | ||
| JSON.stringify(nextConfig, null, 2), | ||
| ); | ||
| if (shouldSkipSystemCronInstall()) { |
There was a problem hiding this comment.
[P2] Don’t let skip mode leave an active stale cron. This early return means a user can set ALPHACLAW_SKIP_SYSTEM_CRON_INSTALL=true, disable sync via /api/sync-cron, and still keep a previously installed /etc/cron.d/openclaw-hourly-sync running. The cron script invokes alphaclaw git-sync unconditionally, so the disabled config is not honored by the already-installed scheduler. We should either remove AlphaClaw's managed cron file when disabling, even in skip mode, or make hourly-git-sync.sh exit when cron/system-sync.json has enabled:false.
Signed-off-by: kesslerio <martin@kessler.io>
|
Thanks for the review. Fixed both.
Added coverage for PATH propagation and the disabled-sync script case. |
…-paths Signed-off-by: kesslerio <martin@kessler.io>
Signed-off-by: kesslerio <martin@kessler.io>
|
@chrysb conflicts are resolved and pushed. I merged current
One extra thing came up after the merge: Checked:
Could you take another look when you get a chance? |
chrysb
left a comment
There was a problem hiding this comment.
Reviewed the refreshed PR. Prior runtime path and cron feedback is addressed, merge conflicts are resolved, and focused/full test suites passed locally.
What
This makes AlphaClaw less picky about where it is allowed to write runtime helper files.
By default, nothing changes. AlphaClaw still uses
/etc/cron.d/openclaw-hourly-sync,/usr/local/bin/git, and the existing askpass helper path. Deployments that run as a non-root user, mount read-only system paths, or manage cron outside the app can now set:ALPHACLAW_SKIP_SYSTEM_CRON_INSTALL=trueto keep AlphaClaw from writing/etc/cron.dALPHACLAW_GIT_SHIM_PATH=/some/writable/bin/gitto move the managed git shimALPHACLAW_GIT_ASKPASS_PATH=/some/writable/path/git-askpass.shto move the askpass helperThe same cron opt-out now applies in startup, onboarding, and the
/api/sync-cronroute. I also fixed the remote config restore helper so sanitized environments still keepPATH; without that, git can disappear even though the caller only meant to avoid passing secrets through.Why
The current defaults are fine for a simple Docker image. They are brittle once AlphaClaw runs in a stricter container, on NixOS-style hosts, or anywhere
/etcand/usr/local/binare owned by the image or supervisor instead of the app process.This keeps the friendly defaults but gives managed deployments a clean escape hatch. No local patching of packaged files, no special entrypoint rewrite, and no surprise write attempt to a host-level cron directory when cron is managed elsewhere.
Evidence
npm test -- tests/server/git-runtime.test.js tests/server/git-shim.test.js tests/server/routes-onboarding.test.js tests/server/routes-system.test.jsnpm testgit diff --checkcodex review --base origin/main, with no blocking findingstest (22)passedReal behavior proof
I tested this against a real AlphaClaw server process, not only mocks.
Runtime setup:
Observed startup output included:
Then I opened the setup UI in a browser, logged in with the test password, and reached the setup choice screen. I also verified that the custom git shim and askpass helper existed and were executable, while the smoke run did not create
/etc/cron.d/openclaw-hourly-sync.What I did not test: a full production onboarding against real GitHub credentials from this branch. The touched behavior is covered by the onboarding route tests and the live smoke above, but I did not create a real workspace repo during the smoke.
AI assistance
Built with Codex. I reviewed the diff, ran the tests above, and verified the runtime behavior locally before opening the PR.