refactor(plugin/{codex,claude-code}): extract installer wrapper to checked-in file#2026
Merged
Merged
Conversation
The installer-emitted codex() wrapper had grown to ~60 lines of shell
function body inlined as a marker-delimited block inside the user's
~/.zshrc / ~/.bashrc. Every upgrade required the awk-strip-and-append
dance, which had a known edge case (rc with begin-marker but no
end-marker) we'd already had to harden against, and the inline noise
was hostile to anyone reading their own rc.
This commit switches to the standard pyenv / nvm / fnm pattern:
- Wrapper body lives in its own file at ~/.openviking/codex-plugin.rc.sh
(path overridable via OPENVIKING_CODEX_WRAPPER_RC). Full overwrite on
every install — no marker logic inside the wrapper file itself.
- The user's shell rc gets a single one-line source hook, still wrapped
in marker comments for cleanup-on-uninstall:
# >>> openviking-codex-plugin >>>
[ -f "$HOME/.openviking/codex-plugin.rc.sh" ] && . "..."
# <<< openviking-codex-plugin <<<
Since the content of this block never changes across installs, the
marker-replacement logic only triggers the legacy-cleanup path once
when upgrading from a pre-rc-split install that inlined the full
wrapper.
User-visible improvements:
- ~/.zshrc OV-plugin block: ~70 lines → 3 lines.
- `cat ~/.openviking/codex-plugin.rc.sh` shows the wrapper directly.
- Uninstall is just `rm ~/.openviking/codex-plugin.rc.sh` + delete the
3-line block — no awk required.
- Upgrades touch the rc file at most once (to install the source hook);
subsequent installs only rewrite the wrapper file.
Verified end-to-end: stale install with the old inline wrapper got the
3-line source hook substituted in place; `source ~/.zshrc && type codex`
showed the new wrapper loaded from the standalone file.
…ance
Follow-up to the rc-split commit. Instead of embedding the wrapper body
as a heredoc inside install.sh and writing it to a copy under
~/.openviking/, the wrapper now lives as its own checked-in file at
examples/codex-memory-plugin/setup-helper/wrapper.sh. The user's shell
rc sources that file directly from the cloned plugin checkout (the path
the installer already manages via git fetch + reset --hard).
What this buys:
- Wrapper diffs are real diffs — code review sees `+ codex() { ... }`
rather than `+ heredoc lines inside install.sh that produce
~/.openviking/codex-plugin.rc.sh`.
- No copy step in the installer means no installer code path for "did
the user accidentally edit ~/.openviking/codex-plugin.rc.sh?" or "is
the copied file in sync with what the installer would produce now?"
- Updates ride for free on `git pull` / the installer's existing
fetch+reset. No "re-run installer to refresh the wrapper" step.
- Uninstall is just `rm ~/.openviking/openviking-repo` (or just leave it
— the source hook will silently no-op when the file is gone, since
it's gated with `[ -f ... ] && .`).
Installer shrinks from ~420 lines (with the inline heredoc) to ~340.
The wrapper is unchanged content-wise; this commit only moves where it
lives.
The installer-emitted claude() wrapper had been inlined as a marker-delimited block in the user's ~/.zshrc / ~/.bashrc. Every upgrade required the awk-strip-and-append dance, the inline noise was hostile to anyone reading their own rc, and there was a known footgun: if the END marker got hand-deleted from the rc, the next install's awk-strip would drop everything from the BEGIN marker to EOF. Switch to the standard pyenv / nvm / fnm pattern, mirroring what the codex-memory-plugin installer now does (see volcengine#2023): - Wrapper body lives in its own file at ~/.openviking/claude-plugin.rc.sh (path overridable via OPENVIKING_CLAUDE_WRAPPER_RC). Full overwrite on every install — no marker logic inside the wrapper file itself. - The user's shell rc gets a single one-line source hook, still marker-wrapped for clean uninstall: # >>> openviking claude-code memory plugin >>> [ -f "$HOME/.openviking/claude-plugin.rc.sh" ] && . "..." # <<< openviking claude-code memory plugin <<< Content is constant across installs, so the marker-replacement logic only triggers the legacy-cleanup path once (when upgrading from a pre-rc-split install that inlined the full claude() function body). User-visible improvements: - ~/.zshrc OV-plugin block: ~16 lines of wrapper body → 3 lines. - Wrapper body is a real file you can `cat` / `diff` / restore from source control; no need to re-run the installer to inspect it. - Uninstall: `rm ~/.openviking/claude-plugin.rc.sh` + delete the 3-line marker block. - The END-marker corruption footgun is gone, since the marker block content is bytestring-stable and the awk-strip only runs when both markers are present anyway. No behavior change to the wrapper itself (still pulls url/api_key from ovcli.conf via jq).
…per/wrapper.sh
Squash-style follow-up to the previous rc-split commit on this branch:
now that the wrapper lives in its own file conceptually, just check it
in at examples/claude-code-memory-plugin/setup-helper/wrapper.sh and
have the user's shell rc source it directly from the cloned plugin
checkout. No copy step, no heredoc dance in install.sh.
Why this is better than the previous approach (wrapper body embedded as
a heredoc in install.sh, written to a copy in ~/.openviking):
- Wrapper is a real reviewable file. Diffs show `+ claude() { ... }`,
not "+ heredoc lines that produce the wrapper".
- Updates ride on the installer's existing `git fetch + reset --hard`
step — no separate "re-run installer to refresh the copy" path.
- One less source of truth (no $HOME copy that can drift from the
installer's intent).
- Uninstall: `rm ~/.openviking/openviking-repo`; the source hook in the
rc silently no-ops via `[ -f ... ] && .`.
The previous commit on this branch already shrunk the rc block from
~16 inline lines to 3 (marker + source hook + marker). This commit just
moves the wrapper body from "embedded in installer" to "checked into the
repo at a stable path", with no behavior change to the wrapper itself.
This was referenced May 13, 2026
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors both the codex and claude-code memory plugin installers so the shell wrapper functions live in checked-in wrapper.sh files, while users’ shell rc files only contain a stable marker block that sources those wrappers from the cloned OpenViking repo checkout.
Changes:
- Extract the previously inlined shell wrapper bodies into
examples/<plugin>/setup-helper/wrapper.sh. - Update both installers to write a minimal 3-line marker block that sources the wrapper from the repo checkout (no
$HOMEcopy). - Adjust rc marker replacement logic to clean up legacy inline-wrapper blocks during upgrades.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| examples/codex-memory-plugin/setup-helper/wrapper.sh | Adds standalone codex() wrapper implementation (moved out of installer heredoc). |
| examples/codex-memory-plugin/setup-helper/install.sh | Switches rc installation to a stable source hook pointing at the checked-in wrapper file. |
| examples/claude-code-memory-plugin/setup-helper/wrapper.sh | Adds standalone claude() wrapper implementation (moved out of installer inline block). |
| examples/claude-code-memory-plugin/setup-helper/install.sh | Switches rc installation to a stable source hook pointing at the checked-in wrapper file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
202
to
212
| touch "$RC" | ||
| if grep -qF "$MARKER_BEGIN" "$RC"; then | ||
| info "Existing wrapper detected in $RC — replacing in place" | ||
| info "Replacing openviking source hook in $RC" | ||
| # Strip existing block (whether it's the new one-liner or an old | ||
| # inline-wrapper block from a previous version). | ||
| awk -v b="$MARKER_BEGIN" -v e="$MARKER_END" ' | ||
| $0 == b {skip=1; next} | ||
| $0 == e {skip=0; next} | ||
| !skip | ||
| ' "$RC" > "$RC.tmp" && mv "$RC.tmp" "$RC" | ||
| else |
Comment on lines
+3
to
+5
| # Installed by examples/codex-memory-plugin/setup-helper/install.sh to | ||
| # ${OPENVIKING_CODEX_WRAPPER_RC:-~/.openviking/codex-plugin.rc.sh}. The | ||
| # installer copies this file verbatim — re-run the installer to update. |
ZaynJarvis
approved these changes
May 13, 2026
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
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.
Summary
Refactor the shell-rc wrappers shipped by both the claude-code and codex memory plugin installers. Same shape applied to both plugins:
heredoc/ inline-rc-block, into a checked-in standalone file atexamples/<plugin>/setup-helper/wrapper.sh.Before / after
Before, installers had ~16–60 lines of shell function body inlined in the user's
~/.zshrc/~/.bashrc, written viaread -r -d '' BODY <<'WRAPPER'heredoc inside install.sh, withawkstrip-and-append on every upgrade. Wrapper diffs showed up as heredoc-content changes inside the installer.After, in both plugins:
setup-helper/wrapper.shis a real, syntax-highlighted, diffable source file.# >>> marker,[ -f ... ] && . "...",# <<< marker).git fetch + reset --hard.install.sh~420 → ~340 lines; claudeinstall.shshed ~16 lines too.Why combined
Same shape, same pattern, identical motivation — easier to review as one PR than two. Originally split into #2024 (claude) and #2025 (codex); folding them into a single PR here. Both predecessor PRs are being closed in favor of this one.
Behavior preserved
Wrapper bodies are byte-for-byte the same as what shipped before — same ovcli.conf parsing, same env injection, same identity headers. Only where the wrapper code lives changes.
For codex specifically (the more involved wrapper): the
.mcp.jsoncache rewriting (URL + bearer_token_env_var sync) and the empty-api-key handling — both already in main via #2023 — are preserved verbatim, just sitting inwrapper.shinstead of being embedded ininstall.sh.Test plan
bash -n/zsh -npass on all four files (twoinstall.sh, twowrapper.sh)source ~/.zshrc && type claudeandtype codexboth show their respective wrappers loaded from the standalone filesclaudeinvocation still injectsOPENVIKING_URL/OPENVIKING_API_KEYfrom ovcli.confcodexinvocation still re-renders cache .mcp.json url + bearer_token_env_var and skips empty env varsrm ~/.openviking/openviking-repono-ops both source hooks (no error, functions just aren't defined)Closes #2024.
Closes #2025.