^R D4: extract Go/Colima host-utility wrappers into scripts/lib/launcher/go-hostutil.sh#396
Conversation
…go-hostutil.sh (byte-identical move, shellcheck/shfmt/bash-n green and module body proven equal via diff/md5 7c6ccafdc006e9b642f40d1e0de603a2; launcher is user-visible but behavior is unchanged) (risk: touches the go_hostutil/go_colimautil primitives the launcher uses to run workcell-hostutil and workcell-colimautil on the host via go run, plus the exit-code recovery and the go_hostutil_publish_pr credential env-var allowlist, so a subtle change could weaken the runtime boundary or leak the sanitised environment; case: pure contiguous move of go_hostutil/run_go_hostutil_preserve_exit/go_hostutil_publish_pr/go_colimautil with zero logic change, dependencies ensure_go_run_env + run_clean_host_command_in_dir + ROOT_DIR/HOST_GO_BIN already sourced or assigned before the new module, first call site well after the source line; the only workcell edit beyond the deletion is a source line, the shell_files allowlist additions, and an SC2034 disable on HOST_GO_BIN whose sole use now lives in the sourced module) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…de execution impact; markdownlint green; supporting documentation for the D4 go-hostutil extraction) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…(the extracted scripts/lib/launcher/go-hostutil.sh is now sourced launcher code, but GenerateControlPlaneManifest in internal/metadatautil/core.go only listed the older host artifacts, so the new module was outside the signed/provenance-verified manifest and go-hostutil changes wouldn't be caught; add it to hostArtifacts and regenerate control-plane-manifest.json) (manifest verify + go test green; supporting the D4 go-hostutil extraction - closes the provenance gap the move opened) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…C2034 (the extracted module is HOST_GO_BIN's sole consumer, so resolving it in scripts/workcell left the var used only across a source=/dev/null boundary that ShellCheck can't see, requiring a SC2034 disable; move the resolve_fixed_host_tool assignment into go-hostutil.sh - resolve_fixed_host_tool is provided by the host-exec.sh module sourced immediately before - so set and use co-locate and the suppression is removed; regenerated the manifest for the module change) (shellcheck -x clean with no disable, behavior-preserving - same resolution, same abort-on-missing-go; supporting the go-hostutil extraction, prefers refactor over alert suppression per repo policy)
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaa477be0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaa477be0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…the module (P1 from Codex on #396) (moving go_hostutil/run_go_hostutil_preserve_exit out of scripts/workcell violated the security ordering invariant that REAL_HOME and WORKCELL_GO_CACHE_ROOT must be established before go_hostutil is defined, and broke three repo-owned validators that pinned those functions to scripts/workcell; behavior-preserving - only the source-line position and the validators' target paths change) Ordering invariant: the go_hostutil/go_colimautil wrappers are defined when scripts/workcell sources scripts/lib/launcher/go-hostutil.sh, so that source line now runs AFTER REAL_HOME="$(resolve_workcell_real_home)" and the WORKCELL_GO_CACHE_ROOT export block (moved down from just after host-exec.sh to immediately after the esac). host-exec.sh and go-run-env.sh stay sourced before, so HOST_GO_BIN resolution in the module still works; nothing between the old and new source position uses go_hostutil or HOST_GO_BIN. Validators retargeted to follow the moved code: - internal/testkit/security_boundary_test.go: the ordering assertion now targets the go-hostutil.sh source-line index (must come after REAL_HOME and WORKCELL_GO_CACHE_ROOT), preserving the existing presence+order checks, and additionally asserts go_hostutil() is defined in the module file. - scripts/verify-invariants.sh: extract_top_level_bash_function for run_go_hostutil_preserve_exit and the go_hostutil-body rg checks now read scripts/lib/launcher/go-hostutil.sh instead of scripts/workcell. - runtime/container/control-plane-manifest.json regenerated for the workcell source-line move. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed the P1. The extraction broke three validators that pin go_hostutil/run_go_hostutil_preserve_exit to scripts/workcell, and tripped the security ordering invariant. Fixes: (1) moved the |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d9c244e10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…-hostutil extraction moved HOST_GO_BIN's resolution into scripts/lib/launcher/go-hostutil.sh, but the contract doc still described it as a global set in scripts/workcell; update the module-contract paragraph to state ROOT_DIR is set in scripts/workcell while HOST_GO_BIN is resolved by the module itself via resolve_fixed_host_tool from host-exec.sh, so the doc matches the implementation) (markdownlint green, doc-only; doc/code lockstep)
|
Fixed the P3: updated launcher-contract.md so HOST_GO_BIN is described as resolved by the go-hostutil.sh module itself (via resolve_fixed_host_tool from host-exec.sh), not set in scripts/workcell — matching the implementation. Only ROOT_DIR remains a workcell-side global for the module. markdownlint green. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
D4 (modularize the launcher) — increment 3 of N. Follows #394 (host-detect) and #395 (host-exec), both merged. Extracts the Go/Colima host-utility wrappers from
scripts/workcellinto a sourced module.What
scripts/lib/launcher/go-hostutil.sh:go_hostutil,run_go_hostutil_preserve_exit,go_hostutil_publish_pr,go_colimautilmoved verbatim (byte-identical, md5-confirmed) fromscripts/workcell(contiguous block; the followingHOST_GIT_BINassignment was correctly left in place).scripts/workcell: block deleted + onesource …/go-hostutil.shline.internal/metadatautil/core.go: module added tohostArtifacts(signed-manifest coverage); manifest regenerated.shell_filesgates; contract section added todocs/launcher-contract.md.HOST_GO_BIN — resolved in-module, no alert suppression
HOST_GO_BINis now consumed only by these wrappers. Rather than leave it assigned inscripts/workcell(where its cross-source=/dev/nulluse is invisible to ShellCheck, forcing an SC2034 disable), itsresolve_fixed_host_toolassignment moves intogo-hostutil.sh—resolve_fixed_host_toolcomes fromhost-exec.sh, sourced immediately before — so set and use co-locate and no suppression is needed (repo policy: refactor over suppressing alerts). Behavior-preserving: same resolution, same abort-on-missing-go.Validation
shellcheck -x(clean, no disables),shfmt -d,bash -n scripts/workcell,go build/go test ./internal/metadatautil, manifest verify, markdownlint — all green.🤖 Generated with Claude Code