Skip to content

fix: install gogcli and add linuxbrew to global PATH#86749

Closed
jarvis-ai-gregmoser wants to merge 1 commit into
openclaw:mainfrom
jarvis-ai-gregmoser:fix-sandbox-brew-gog
Closed

fix: install gogcli and add linuxbrew to global PATH#86749
jarvis-ai-gregmoser wants to merge 1 commit into
openclaw:mainfrom
jarvis-ai-gregmoser:fix-sandbox-brew-gog

Conversation

@jarvis-ai-gregmoser

Copy link
Copy Markdown
Contributor

Fixes missing gogcli and PATH issues for sandbox image.

@openclaw-barnacle openclaw-barnacle Bot added docker Docker and sandbox tooling size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 26, 2026
@clawsweeper

clawsweeper Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Thanks for the contribution. I reviewed the branch, and this PR is not a good landing base for OpenClaw.

Close this PR: it targets an obsolete root Dockerfile path, is currently conflicting, and broadens the common sandbox image dependency/PATH behavior without proof or maintainer-confirmed default-image direction. The separate current gog onboarding failure is already tracked by a cleaner open fix candidate.

So I’m closing this PR rather than keeping an unmergeable branch open. A new narrow PR that carries only the useful part is welcome.

Review details

Best possible solution:

Close this stale branch; land the focused gog skill metadata fix in #95019 for the current install failure, and use a maintainer-sponsored current-Dockerfile PR only if gogcli should become a common-image default.

Do we have a high-confidence way to reproduce the issue?

No high-confidence runtime reproduction is present. Source inspection shows the current common image does not install gogcli, but the PR does not establish that the shared common image is expected to include it or provide Docker build/run proof.

Is this the best way to solve the issue?

No. As submitted, this is not the best fix because it edits an obsolete path and broadens common-image defaults; the current install failure is better handled by the focused skill metadata PR, while any default-image addition needs a new maintainer-approved current-Dockerfile proposal.

Security review:

Security review needs attention: The diff adds a new Homebrew package install to the common sandbox image, which needs supply-chain and build reliability acceptance before any replacement lands.

  • [low] Review the new image package source — Dockerfile.sandbox-common:45
    brew install gogcli would make the common sandbox image depend on live Homebrew formula resolution during build; maintainers should accept that supply-chain surface or choose a pinned/checksummed install path in a new current-Dockerfile PR.
    Confidence: 0.82

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • vincentkoc: Recent/current history attributes the active common sandbox Dockerfile and setup script path to Vincent Koc's Docker/release work. (role: current path contributor; confidence: high; commits: 844f405ac1be, 459edec9badc, 6d5e142b9312; files: scripts/docker/sandbox/Dockerfile.common, scripts/sandbox-common-setup.sh, Dockerfile.sandbox-common)
  • steipete: History shows Peter Steinberger introduced the old root Dockerfile.sandbox-common surface that this PR edits. (role: original sandbox common contributor; confidence: high; commits: 852c8979563c; files: Dockerfile.sandbox-common, scripts/sandbox-common-setup.sh)
  • jacobtomlinson: Jacob Tomlinson changed the old sandbox common Dockerfile in earlier broad Dockerfile hardening work, making them relevant context for default-image changes. (role: adjacent Docker hardening contributor; confidence: medium; commits: 63802c111288; files: Dockerfile.sandbox-common)

Codex review notes: model internal, reasoning high; reviewed against f8675b3b70d3.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels May 26, 2026
@clawsweeper

clawsweeper Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@clawsweeper

clawsweeper Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docker Docker and sandbox tooling merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants