Skip to content

Pin gateway setup to LKG and automate drift updates - #550

Closed
ranjeshj wants to merge 5 commits into
openclaw:masterfrom
ranjeshj:test/lkg-updater-e2e
Closed

Pin gateway setup to LKG and automate drift updates#550
ranjeshj wants to merge 5 commits into
openclaw:masterfrom
ranjeshj:test/lkg-updater-e2e

Conversation

@ranjeshj

Copy link
Copy Markdown
Collaborator

Summary

This PR makes gateway install/testing deterministic by pinning SetupEngine to a repository-controlled Last Known Good (LKG) gateway version, then adds CI + automation to detect and advance that pin through a standing draft update PR.

What changed

  1. Single source of truth for gateway LKG

    • Added src/OpenClaw.SetupEngine/GatewayLkgVersion.cs with embedded LkgVersion constant.
    • Setup CLI (Program.cs) and Setup UI (SetupWindow.xaml.cs) now apply LKG defaults via GatewayLkgVersion.ApplyToConfig(...).
  2. SetupEngine install command supports explicit version pinning

    • InstallCliStep now builds install command via BuildInstallCommand(...).
    • When version is set, SetupEngine runs:
      • curl ... | bash -s -- --version '<version>'
    • When unset, behavior remains:
      • curl ... | bash
    • Added shell escaping + newline rejection for version safety.
  3. E2E setup aligns with SetupEngine LKG source

    • E2E fixture now writes Gateway.Version from GatewayLkgVersion.ResolveLkgVersion().
  4. CI drift signal in existing Build/Test workflow

    • Added drift detection step in .github/workflows/ci.yml existing test job.
    • Compares pinned LKG vs npm openclaw@latest.
    • Drift is visible in checks (warning + job summary + continue-on-error step failure), without blocking the full workflow.
  5. Automated LKG bump workflow

    • Added .github/workflows/gateway-lkg-update.yml.
    • Daily schedule + manual dispatch.
    • Detects drift, updates GatewayLkgVersion.cs, force-pushes deterministic branch automation/gateway-lkg-update, and creates/updates one standing draft PR.
  6. Tests and docs

    • Added GatewayLkgVersionTests.
    • Extended SetupStepsTests for versioned install command behavior.
    • Updated DEVELOPMENT.md with LKG workflow and operations.

Follow-up hardening from dual-model (Hanselman) review

Addressed both review findings:

  1. Custom installer compatibility regression prevention

    • LKG auto-apply now only occurs for default installer URL.
    • If Gateway.InstallUrl is custom, LKG is not auto-injected unless version is explicitly set.
    • Added test coverage for this behavior.
  2. Updater/CI version format validation

    • Added strict version-format validation for npm latest in CI drift step.
    • Added strict validation for both pinned and latest versions in updater workflow before source rewrite.

Testing performed

Required local validation (run successfully)

  • ./build.ps1
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore

Fork workflow validation (end-to-end)

Drift/PR behavior was validated in fork branch test/lkg-updater-e2e:

Notes for reviewers

  • LKG in this PR is currently pinned to 2026.5.22.
  • This PR intentionally keeps drift signaling non-blocking while still highly visible.
  • Updater workflow is deterministic and reuses a single draft PR to avoid PR spam.

ranjeshj and others added 2 commits May 26, 2026 19:40
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper

clawsweeper Bot commented May 27, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed May 28, 2026, 10:18 AM ET / 14:18 UTC.

Summary
The PR adds a SetupEngine gateway LKG version source, applies it to CLI/UI/E2E setup defaults, passes --version to the installer, adds tests/docs, and adds CI plus scheduled workflows for drift detection and LKG bump PRs.

Reproducibility: not applicable. as a feature/operational PR rather than a bug report. The relevant verification is source review plus the contributor's setup E2E and workflow proof, which covers the changed runtime path.

Review metrics: 2 noteworthy metrics.

  • Diff size: 11 files, +331/-7. The patch spans setup runtime, tests, docs, E2E config, and repository automation rather than one isolated code path.
  • Workflow surface: 1 added, 1 modified. The PR adds a scheduled write-permission updater and changes CI drift reporting, which need maintainer approval beyond unit tests.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Maintainers should explicitly approve or reject the LKG-as-default setup policy.
  • [P2] Maintainers should explicitly approve or split the scheduled updater workflow before merge.

Risk before merge

  • [P1] Default setup semantics change: a null/default Gateway.Version no longer tracks latest for the default installer URL, so existing setup configs relying on latest-by-default will receive the repository LKG instead.
  • [P1] The new scheduled workflow has contents and pull-requests write permissions and force-updates a deterministic branch from npm registry metadata; the version validation narrows the risk, but maintainers still need to approve that automation model.
  • [P2] The CI drift check depends on live npm registry availability and is intentionally non-blocking, so it is a visibility signal rather than an enforcement gate.

Maintainer options:

  1. Approve the LKG operating model
    Merge as-is if maintainers want default installer runs to use the repository LKG and want the scheduled workflow to maintain one standing draft bump PR.
  2. Preserve latest-by-default compatibility
    Change the implementation so existing null/default Gateway.Version behavior still installs latest, with LKG enabled by an explicit setting or setup mode.
  3. Defer write-permission automation
    Pause or split the updater workflow if maintainers want the setup pin first but need a separate policy review for scheduled force-push automation.

Next step before merge

  • [P2] Manual review is needed because the remaining blocker is acceptance of the LKG-default and scheduled branch-updater policies, not a narrow repair ClawSweeper can safely make.

Security
Cleared: The diff adds a write-permission workflow, but it validates registry versions tightly and only rewrites one source constant; I found no concrete security or supply-chain regression.

Review details

Best possible solution:

Land this only with maintainer approval that default SetupEngine installs should use the LKG pin and that the scheduled workflow should own the standing bump PR; otherwise preserve latest-by-default and make LKG opt-in.

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

Not applicable as a feature/operational PR rather than a bug report. The relevant verification is source review plus the contributor's setup E2E and workflow proof, which covers the changed runtime path.

Is this the best way to solve the issue?

Yes if maintainers want LKG as the default setup policy: the PR centralizes the pin, preserves custom installer URLs, tests installer quoting, and proves the pairing-drain path. If latest-by-default compatibility is required, the safer solution is to make LKG opt-in or gated by an explicit setup mode.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 2d1cfb7773d8.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR discussion includes fork updater workflow validation plus copied live output for required validation and repeated win-arm64 setup E2E runs after the LKG and pairing fixes.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR discussion includes fork updater workflow validation plus copied live output for required validation and repeated win-arm64 setup E2E runs after the LKG and pairing fixes.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a meaningful setup reliability and automation improvement with limited but real default-behavior and CI policy impact.
  • merge-risk: 🚨 compatibility: Merging changes default Gateway.Version null behavior from latest-by-default to the repository LKG for default installer setups.
  • merge-risk: 🚨 automation: Merging adds a scheduled GitHub Actions workflow with contents/pull-request write permissions that force-updates a standing branch.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR discussion includes fork updater workflow validation plus copied live output for required validation and repeated win-arm64 setup E2E runs after the LKG and pairing fixes.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR discussion includes fork updater workflow validation plus copied live output for required validation and repeated win-arm64 setup E2E runs after the LKG and pairing fixes.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read fully; its setup/pairing documentation guidance and validation expectations were applied to the PR review, while the read-only review contract prevented running build/tests locally. (AGENTS.md:1, 2d1cfb7773d8)
  • Current main still uses latest-by-default setup: On current main, the setup install step falls back directly to https://openclaw.ai/install-cli.sh and pipes it to bash without a version argument, so the PR's LKG behavior is not already implemented. (src/OpenClaw.SetupEngine/SetupSteps.cs:721, 2d1cfb7773d8)
  • Current config documents null as latest: On current main, default-config.json still documents Gateway.Version null as latest, confirming the PR changes the default setup semantics rather than duplicating existing behavior. (src/OpenClaw.SetupEngine/default-config.json:76, 2d1cfb7773d8)
  • PR centralizes and applies the LKG pin: The PR head adds GatewayLkgVersion with LkgVersion 2026.5.27 and applies it only when Gateway.Version is unset and the installer URL is the default installer URL. (src/OpenClaw.SetupEngine/GatewayLkgVersion.cs:5, d31572700607)
  • PR builds versioned installer command safely: The PR head builds the installer command through BuildInstallCommand, rejects newline-containing versions, shell-escapes URL/version literals, and appends bash -s -- --version when a version is configured. (src/OpenClaw.SetupEngine/SetupSteps.cs:773, d31572700607)
  • PR adds write-permission updater automation: The added workflow grants contents and pull-requests write permissions, validates npm latest/pinned versions, force-pushes automation/gateway-lkg-update, and creates or updates a standing draft PR. (.github/workflows/gateway-lkg-update.yml:8, d31572700607)

Likely related people:

  • ranjeshj: The current SetupEngine install, pairing, drain, default config, and setup tests appear to date to the merged SetupEngine redesign commit, and this PR also builds on that same area. (role: introduced behavior and recent area contributor; confidence: high; commits: cefce3952ab1; files: src/OpenClaw.SetupEngine/SetupSteps.cs, src/OpenClaw.SetupEngine/Program.cs, src/OpenClaw.SetupEngine/default-config.json)
  • Scott Hanselman: Git history shows recent expansion of the CI test workflow that this PR modifies for LKG drift detection. (role: CI workflow area contributor; confidence: medium; commits: 6711591b5c14, b41fe8243cc3; files: .github/workflows/ci.yml)
  • Vincent Koc: The most recent current-main touch to the CI workflow was a CodeQL-related workflow maintenance commit shortly before this PR review. (role: recent CI workflow contributor; confidence: low; commits: b906e1ba8f00; files: .github/workflows/ci.yml)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@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 priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels May 27, 2026
@clawsweeper

clawsweeper Bot commented May 27, 2026

Copy link
Copy Markdown

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.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ranjeshj

Copy link
Copy Markdown
Collaborator Author

Addressed review feedback in commit 97a23d2:

  1. Docs job name fixed
  • Updated DEVELOPMENT.md to reference the correct drift-check location (test job, not e2etests).
  1. Install command quoting behavior proven
  • Kept BuildInstallCommand behavior and added explicit escaping proof coverage:
    • InstallCli_BuildInstallCommand_EscapesSingleQuotesInUrlAndVersion
  • This demonstrates the generated command uses single-quoted shell literals with correct \'' escaping for embedded apostrophes.

Focused proof run

Executed:

  • dotnet test ./tests/OpenClaw.SetupEngine.Tests/OpenClaw.SetupEngine.Tests.csproj --filter "FullyQualifiedName~InstallCli_BuildInstallCommand"

Result:

  • Passed: 3, Failed: 0, Skipped: 0 (includes the new escaping proof test)

Full required validation (already rerun on latest branch state)

  • ./build.ps1
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 27, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ranjeshj
ranjeshj marked this pull request as ready for review May 28, 2026 13:17
@ranjeshj

Copy link
Copy Markdown
Collaborator Author

Verified locally after bumping GatewayLkgVersion to 2026.5.26:

  • ./build.ps1
  • dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore --tl:off
  • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore --tl:off
  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore --tl:off
  • OPENCLAW_RUN_E2E=1 on win-arm64: dotnet test .\tests\OpenClaw.E2ETests\OpenClaw.E2ETests.csproj --no-build -c Debug -r win-arm64 --verbosity normal --tl:off
    • Run 1: 6/6 passed
    • Run 2: 6/6 passed
    • Run 3: 6/6 passed

Drain pending WSL device approvals before pair-node so the WSL CLI's own scope-upgrade request no longer blocks approval of the Windows node request. Keep the final verify drain for any remaining device or node approvals.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ranjeshj

ranjeshj commented May 28, 2026

Copy link
Copy Markdown
Collaborator Author

Implemented a surgical fix for the 2026.5.27 pairing regression.

Regression details:

  • Before the bump, SetupEngine's existing flow paired the operator, then in pair-node auto-approved the Windows node pairing request by running openclaw devices approve "$OPENCLAW_APPROVAL_REQUEST_ID" --json inside WSL.
  • That ordering worked with LKG 2026.5.26; the Windows node request was approved during pair-node, and any extra pending approvals created by the WSL CLI itself were drained later in verify-e2e. We had already run the 2026.5.26 E2E flow successfully 3x with that behavior.
  • With 2026.5.27, the same WSL CLI path started surfacing its own pending scope-upgrade/device approval earlier. Instead of approving the Windows node request, openclaw devices approve failed with GatewayClientRequestError: scope upgrade pending approval.
  • That meant pair-node failed before the later verify-e2e cleanup could run, so the E2E fixture exited during setup.

Fix details:

  • Extracted the pending device-approval drain in VerifyEndToEndStep into a shared helper.
  • Call that helper before pair-node starts, so the WSL CLI's own scope-upgrade/device approval is cleared before SetupEngine tries to approve the Windows node request.
  • Kept the existing final drain in verify-e2e for any remaining device or node approvals.

Validation performed:

  • ./build.ps1
  • dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore --tl:off
  • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore --tl:off
  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore --tl:off
  • dotnet build .\tests\OpenClaw.E2ETests\OpenClaw.E2ETests.csproj -c Debug -r win-arm64 --tl:off
  • OPENCLAW_RUN_E2E=1 dotnet test .\tests\OpenClaw.E2ETests\OpenClaw.E2ETests.csproj --no-build -c Debug -r win-arm64 ...
    • LKG 2026.5.27: 6/6 passed
    • LKG 2026.5.26: 6/6 passed

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed 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. labels May 28, 2026
@shanselman

Copy link
Copy Markdown
Collaborator

Landed in master via the integrated Inno installer stack on branch shanselman/inno-installer-plan (master now at 8edf4eb). Closing this source PR as absorbed.

@shanselman shanselman closed this May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants