chore: fix assorted PR #756 review nits#786
Merged
Merged
Conversation
…checksum-tamper fallback, DNS-label overflow, safety-decline exit code) 1. monetizeapi.RemoveAgentIdentityRegistration: allocate a fresh slice instead of filtering status.Registrations in place, so a caller passing an informer-cached object can't have its backing array silently corrupted. 2. tunnel.RefreshStorefront: no-op quietly when storefrontHostnames has nothing to report (no persistent tunnel/hostname state yet), instead of calling CreateStorefront with zero hostnames and surfacing its "requires at least one hostname" error as a confusing warning on a normal first `obol sell ... --hostname X --no-register`. EnsureTunnelForSell reconciles the storefront once the tunnel exists. 3. obolup.sh: verify_release_checksum now returns exit code 2 for a verified checksum MISMATCH (tamper), distinct from exit code 1 for "couldn't verify" (SHA256SUMS unpublished / no sha tool / no checksum entry). download_release propagates the distinction, and install_obol_binary hard-aborts on a mismatch instead of falling back to `git clone` over the same (potentially compromised) channel. Also fixes the misleading "Release not found" message on a mismatch. 4. agentruntime.MaxIDLength + openclaw/hermes Onboard: bound the deployment id to (63 - the runtime's DNS-label prefix/suffix) at the two Onboard call sites, so "openclaw-<id>" / "hermes-<id>" / "hermes-<id>-ui" can't exceed the 63-char DNS label limit and fail later with an opaque k8s error. validate.Name itself is untouched for other callers. 5. stack.destroyOldBackendIfSwitching: a declined backend-switch prompt during `stack init --force` now returns nil (clean exit 0), matching Down/Purge's behavior for the same ConfirmRunningServicesLoss decline. Previously it returned errSafetyAborted, whose doc comment claimed cmd/obol/stack.go handled it specially — it never did. Comment fixed to reflect that nothing consumes the sentinel today. Deviation: no automated test for (5)'s behavior change — reaching the declined-prompt branch requires ui.UI.IsTTY()==true, which has no test-injectable seam (isTTY is unexported, set from a real isatty.IsTerminal check, no pty dependency in the repo). Verified by inspection + go build/vet/test instead; the change is a single-line return-value swap in an already-covered function. Claude-Session: https://claude.ai/code/session_014YjPMViNrZ7zBVgUQzwEKk
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Small independent nits from the PR #756 review: RemoveAgentIdentityRegistration allocates instead of mutating the caller's backing array; RefreshStorefront no-ops quietly before a persistent tunnel exists; obolup.sh aborts hard on a verified checksum mismatch instead of falling back to a source build in the same attacker-controlled channel; agent --id length bounded to fit the derived k8s prefix; stack init decline stops Init cleanly (a regression where the decline let Init continue past the destroy was caught in review and fixed).
https://claude.ai/code/session_014YjPMViNrZ7zBVgUQzwEKk