ci: enforce RLMX release channel contract#103
Conversation
📝 WalkthroughWalkthroughThis PR establishes RLMX as a git-installed CLI distributed via a source repository while publishing only the SDK to npm. It adds ChangesGit-Based CLI Distribution with Self-Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a new rlmx update command to update git-installed checkouts, transitions the npm package to be SDK-only by removing the bin field, and adds a canonical installer script (scripts/install.sh) along with smoke tests. Feedback on the changes suggests using checkout -f in the installer script to ensure that refreshing an existing checkout with local modifications does not cause the installation to fail.
| if [ -d "$RLMX_INSTALL_DIR/.git" ]; then | ||
| echo "==> Existing checkout found; refreshing" | ||
| git -C "$RLMX_INSTALL_DIR" fetch origin "$RLMX_BRANCH" --tags | ||
| git -C "$RLMX_INSTALL_DIR" checkout "$RLMX_BRANCH" |
There was a problem hiding this comment.
If the existing checkout has any local modifications or untracked files that conflict, git checkout without the force flag will fail and abort the installation. Using checkout -f ensures that the checkout succeeds by discarding local changes, which is the expected behavior for a clean managed refresh.
| git -C "$RLMX_INSTALL_DIR" checkout "$RLMX_BRANCH" | |
| git -C "$RLMX_INSTALL_DIR" checkout -f "$RLMX_BRANCH" |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62f2a0be57
ℹ️ 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".
| echo "==> Installing dependencies" | ||
| npm ci | ||
|
|
||
| echo "==> Building" | ||
| npm run build |
There was a problem hiding this comment.
Install dev dependencies before building
When this installer runs in an environment with NODE_ENV=production, npm ci omits dev dependencies by default (checked with NODE_ENV=production npm config get omit, which returns dev), so the following npm run build cannot find dev-only tools like typescript/tsc. That makes fresh installs fail on production-configured hosts even though the committed dist/ exists; the same pattern also appears in rlmx update, so force dev dependencies for the managed build path (for example npm ci --include=dev or clearing omit).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/install.sh`:
- Around line 17-21: The refresh path uses the existing origin instead of the
requested RLMX_REPO_URL; update the script to read the current origin URL (git
-C "$RLMX_INSTALL_DIR" remote get-url origin), compare it to $RLMX_REPO_URL, and
if they differ either set the origin to the requested URL (git -C
"$RLMX_INSTALL_DIR" remote set-url origin "$RLMX_REPO_URL") before
fetch/checkout/reset or exit with a clear error; ensure this logic surrounds the
existing git -C "$RLMX_INSTALL_DIR" fetch/checkout/reset calls that reference
origin and uses the RLMX_* variables named in the diff.
In `@scripts/smoke-install-update.sh`:
- Around line 53-56: The npm pack check depends on the current working
directory; update the script so the npm pack --json --dry-run invocation runs
from the repository root instead of the caller's cwd. Specifically, wrap or
prefix the existing npm pack --json --dry-run | node ... pipeline (the line
containing "npm pack --json --dry-run" and the inline node check) so it executes
in "$ROOT" (for example by cd'ing into "$ROOT" before running the pipeline or
using a subshell that changes directory), ensuring the SDK-only bin assertion
always inspects the repo root package.
In `@src/cli.ts`:
- Around line 929-945: The update flow currently runs runGit(root, ["reset",
"--hard", "origin/main"]) but doesn't remove untracked files, so using --force
won't produce a fully clean checkout; call the git clean step (e.g.,
runGit(root, ["clean", "-fdx"]) or at minimum ["clean", "-fd"]) before the reset
when force is true (or when you decide to override a dirty state) to remove
untracked files and dirs; locate the update logic around the dirty/force checks
and add the runGit("clean", ...) call before runGit(... "reset", "--hard",
"origin/main") so the working tree truly matches origin/main before
runCommand(root, "npm", ["ci"]).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5d4c191-ecc0-4d51-99e7-fe3799ca29ae
⛔ Files ignored due to path filters (3)
dist/src/cli.jsis excluded by!**/dist/**dist/src/schema.jsis excluded by!**/dist/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.github/workflows/ci.yml.github/workflows/release.yml.github/workflows/rolling-pr.yml.github/workflows/version.ymldocs/release-contract.mdpackage.jsonscripts/install.shscripts/smoke-install-update.shsrc/cli.tssrc/schema.ts
| if [ -d "$RLMX_INSTALL_DIR/.git" ]; then | ||
| echo "==> Existing checkout found; refreshing" | ||
| git -C "$RLMX_INSTALL_DIR" fetch origin "$RLMX_BRANCH" --tags | ||
| git -C "$RLMX_INSTALL_DIR" checkout "$RLMX_BRANCH" | ||
| git -C "$RLMX_INSTALL_DIR" reset --hard "origin/$RLMX_BRANCH" |
There was a problem hiding this comment.
Existing installs ignore RLMX_REPO_URL.
On the refresh path, the script always fetches from the checkout’s current origin, so rerunning the installer with a different RLMX_REPO_URL silently keeps using the old remote while the banner says otherwise. Either update origin first or fail if it does not match the requested repo.
Suggested fix
if [ -d "$RLMX_INSTALL_DIR/.git" ]; then
echo "==> Existing checkout found; refreshing"
+ git -C "$RLMX_INSTALL_DIR" remote set-url origin "$RLMX_REPO_URL"
git -C "$RLMX_INSTALL_DIR" fetch origin "$RLMX_BRANCH" --tags
git -C "$RLMX_INSTALL_DIR" checkout "$RLMX_BRANCH"
git -C "$RLMX_INSTALL_DIR" reset --hard "origin/$RLMX_BRANCH"
else📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -d "$RLMX_INSTALL_DIR/.git" ]; then | |
| echo "==> Existing checkout found; refreshing" | |
| git -C "$RLMX_INSTALL_DIR" fetch origin "$RLMX_BRANCH" --tags | |
| git -C "$RLMX_INSTALL_DIR" checkout "$RLMX_BRANCH" | |
| git -C "$RLMX_INSTALL_DIR" reset --hard "origin/$RLMX_BRANCH" | |
| if [ -d "$RLMX_INSTALL_DIR/.git" ]; then | |
| echo "==> Existing checkout found; refreshing" | |
| git -C "$RLMX_INSTALL_DIR" remote set-url origin "$RLMX_REPO_URL" | |
| git -C "$RLMX_INSTALL_DIR" fetch origin "$RLMX_BRANCH" --tags | |
| git -C "$RLMX_INSTALL_DIR" checkout "$RLMX_BRANCH" | |
| git -C "$RLMX_INSTALL_DIR" reset --hard "origin/$RLMX_BRANCH" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/install.sh` around lines 17 - 21, The refresh path uses the existing
origin instead of the requested RLMX_REPO_URL; update the script to read the
current origin URL (git -C "$RLMX_INSTALL_DIR" remote get-url origin), compare
it to $RLMX_REPO_URL, and if they differ either set the origin to the requested
URL (git -C "$RLMX_INSTALL_DIR" remote set-url origin "$RLMX_REPO_URL") before
fetch/checkout/reset or exit with a clear error; ensure this logic surrounds the
existing git -C "$RLMX_INSTALL_DIR" fetch/checkout/reset calls that reference
origin and uses the RLMX_* variables named in the diff.
| npm pack --json --dry-run | node -e 'let s=""; process.stdin.on("data",d=>s+=d); process.stdin.on("end",()=>{ const p=JSON.parse(s)[0]; if (p.bin) { console.error("npm package exposes bin unexpectedly", p.bin); process.exit(1); } });' || { | ||
| echo "::error::npm package must remain SDK-only and expose no bin" >&2 | ||
| exit 1 | ||
| } |
There was a problem hiding this comment.
Run the SDK-only check from "$ROOT".
This assertion currently depends on the caller’s working directory. If someone invokes the script from outside the repo root, npm pack --dry-run inspects the wrong package.
Suggested fix
-npm pack --json --dry-run | node -e 'let s=""; process.stdin.on("data",d=>s+=d); process.stdin.on("end",()=>{ const p=JSON.parse(s)[0]; if (p.bin) { console.error("npm package exposes bin unexpectedly", p.bin); process.exit(1); } });' || {
+(cd "$ROOT" && npm pack --json --dry-run) | node -e 'let s=""; process.stdin.on("data",d=>s+=d); process.stdin.on("end",()=>{ const p=JSON.parse(s)[0]; if (p.bin) { console.error("npm package exposes bin unexpectedly", p.bin); process.exit(1); } });' || {
echo "::error::npm package must remain SDK-only and expose no bin" >&2
exit 1
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| npm pack --json --dry-run | node -e 'let s=""; process.stdin.on("data",d=>s+=d); process.stdin.on("end",()=>{ const p=JSON.parse(s)[0]; if (p.bin) { console.error("npm package exposes bin unexpectedly", p.bin); process.exit(1); } });' || { | |
| echo "::error::npm package must remain SDK-only and expose no bin" >&2 | |
| exit 1 | |
| } | |
| (cd "$ROOT" && npm pack --json --dry-run) | node -e 'let s=""; process.stdin.on("data",d=>s+=d); process.stdin.on("end",()=>{ const p=JSON.parse(s)[0]; if (p.bin) { console.error("npm package exposes bin unexpectedly", p.bin); process.exit(1); } });' || { | |
| echo "::error::npm package must remain SDK-only and expose no bin" >&2 | |
| exit 1 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/smoke-install-update.sh` around lines 53 - 56, The npm pack check
depends on the current working directory; update the script so the npm pack
--json --dry-run invocation runs from the repository root instead of the
caller's cwd. Specifically, wrap or prefix the existing npm pack --json
--dry-run | node ... pipeline (the line containing "npm pack --json --dry-run"
and the inline node check) so it executes in "$ROOT" (for example by cd'ing into
"$ROOT" before running the pipeline or using a subshell that changes directory),
ensuring the SDK-only bin assertion always inspects the repo root package.
| if (dirty && !force) { | ||
| throw new Error("Refusing to update with local changes. Commit/stash them or rerun with --force for managed installs."); | ||
| } | ||
|
|
||
| console.log(`rlmx update: ${root}`); | ||
| console.log(`before: ${before}`); | ||
| runGit(root, ["fetch", "origin", "main", "--tags"]); | ||
| const target = runGit(root, ["rev-parse", "origin/main"]); | ||
| console.log(`target: ${target}`); | ||
|
|
||
| if (before === target && !dirty) { | ||
| console.log("Already up to date."); | ||
| return; | ||
| } | ||
|
|
||
| runGit(root, ["reset", "--hard", "origin/main"]); | ||
| runCommand(root, "npm", ["ci"]); |
There was a problem hiding this comment.
--force does not fully restore a clean checkout.
This path only does git reset --hard, which drops tracked edits but leaves untracked files behind. A checkout that is dirty because of untracked files will still not match origin/main after rlmx update --force, despite the help/schema text saying it resets the managed checkout. Add a clean step before rebuilding.
Suggested fix
if (before === target && !dirty) {
console.log("Already up to date.");
return;
}
runGit(root, ["reset", "--hard", "origin/main"]);
+ runGit(root, ["clean", "-fd"]);
runCommand(root, "npm", ["ci"]);
runCommand(root, "npm", ["run", "build"]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (dirty && !force) { | |
| throw new Error("Refusing to update with local changes. Commit/stash them or rerun with --force for managed installs."); | |
| } | |
| console.log(`rlmx update: ${root}`); | |
| console.log(`before: ${before}`); | |
| runGit(root, ["fetch", "origin", "main", "--tags"]); | |
| const target = runGit(root, ["rev-parse", "origin/main"]); | |
| console.log(`target: ${target}`); | |
| if (before === target && !dirty) { | |
| console.log("Already up to date."); | |
| return; | |
| } | |
| runGit(root, ["reset", "--hard", "origin/main"]); | |
| runCommand(root, "npm", ["ci"]); | |
| if (dirty && !force) { | |
| throw new Error("Refusing to update with local changes. Commit/stash them or rerun with --force for managed installs."); | |
| } | |
| console.log(`rlmx update: ${root}`); | |
| console.log(`before: ${before}`); | |
| runGit(root, ["fetch", "origin", "main", "--tags"]); | |
| const target = runGit(root, ["rev-parse", "origin/main"]); | |
| console.log(`target: ${target}`); | |
| if (before === target && !dirty) { | |
| console.log("Already up to date."); | |
| return; | |
| } | |
| runGit(root, ["reset", "--hard", "origin/main"]); | |
| runGit(root, ["clean", "-fd"]); | |
| runCommand(root, "npm", ["ci"]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli.ts` around lines 929 - 945, The update flow currently runs
runGit(root, ["reset", "--hard", "origin/main"]) but doesn't remove untracked
files, so using --force won't produce a fully clean checkout; call the git clean
step (e.g., runGit(root, ["clean", "-fdx"]) or at minimum ["clean", "-fd"])
before the reset when force is true (or when you decide to override a dirty
state) to remove untracked files and dirs; locate the update logic around the
dirty/force checks and add the runGit("clean", ...) call before runGit(...
"reset", "--hard", "origin/main") so the working tree truly matches origin/main
before runCommand(root, "npm", ["ci"]).
Summary
Implements the RLMX release-channel contract:
scripts/install.sh+rlmx updateare the canonical CLI/application install/update path.bin.mainis documented as the application release boundary.scripts/install.sh,rlmx updatedirty-check refusal, and update happy path against a temporary localmainremote.Verification
npm run buildnpm run checknpm test— 377 passing, 0 failingbash scripts/smoke-install-update.shnpm pack --json --dry-run—hasBin=falseSummary by CodeRabbit
New Features
rlmx updatecommand to update managed installations with optional force flag to override safety checks.Documentation
Chores