fix(update): restart pm2 genie-serve on stale-inode + truthful verify probe - #1675
Conversation
… probe
`genie update` left the running pm2 `genie-serve` process serving pre-update
bytes from a deleted node_modules inode (visible via `/proc/<pid>/cwd ->
.../.old-<hash> (deleted)`) until the operator manually ran
`pm2 restart genie-serve`. The verify probe couldn't surface it because
`readServerHealth` was a tautology — it returned `{ version: VERSION }` from
the calling CLI's compile-time constant rather than probing the daemon's
actual state.
Two-pronged fix:
1. `readServerHealth` now reads `/proc/<pid>/cwd` (Linux) to detect the
kernel `(deleted)` marker, AND reads the on-disk `package.json` for the
true installed version. Non-Linux platforms degrade gracefully (no
inode probe, optimistic same-version inference).
2. New `restartServeIfStale` runs in `runPostUpdateMaintenanceSafe` BEFORE
the verify probe. Detects pm2 + `genie-serve` + deleted-inode cwd, then
issues `pm2 restart genie-serve --update-env` and polls for a new pid.
No-op when pm2 is missing, no genie-serve entry, or daemon already on
live inode.
New `daemon-stale-inode` `VerifyResult` variant carries pid + cwd + the
`pm2 restart` remediation in the banner. Exits 1 (alongside
`health-unreachable`) when verify catches a still-stale state — CI /
scripted updaters now see a hard failure instead of a silent half-update.
Tests:
- `decideVerify` precedence: inode-stale wins over version-mismatch
- post-restart `ok` path regression lock
- banner contains pid, cwd `(deleted)` marker, and pm2 restart string
- source-shape locks: restart-before-verify ordering, exit-code 1 path,
Linux-gated `/proc` probe, pm2 `jlist` parsing
57 existing update tests + 5 new variants pass.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements detection and automatic remediation for stale daemon inodes, a condition where the genie-serve process continues running from a deleted directory after a package update. The changes include adding Linux-specific probes via /proc, integrating an automatic PM2 restart into the maintenance lifecycle, and updating the verification banner with actionable fix instructions. Feedback focused on a performance optimization for the readInstalledPackageVersion function, recommending memoization to avoid the overhead of repeated synchronous shell executions during health polling cycles.
| function readInstalledPackageVersion(): string | null { | ||
| const candidates: string[] = [ | ||
| join(homedir(), '.bun', 'install', 'global', 'node_modules', '@automagik', 'genie', 'package.json'), | ||
| ]; | ||
| const npmPrefix = safeExec('npm prefix -g', 1500); | ||
| if (npmPrefix) { | ||
| candidates.push(join(npmPrefix, 'lib', 'node_modules', '@automagik', 'genie', 'package.json')); | ||
| } | ||
| for (const path of candidates) { | ||
| try { | ||
| const pkg = JSON.parse(readFileSync(path, 'utf-8')) as { version?: unknown }; | ||
| if (typeof pkg.version === 'string' && /^\d+\.\d+/.test(pkg.version)) return pkg.version; | ||
| } catch { | ||
| // try next candidate | ||
| } | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The readInstalledPackageVersion function performs a synchronous shell execution (npm prefix -g) which can be relatively slow (often 100ms+). Since this function is called inside readServerHealth, which in turn is called within the pollHealth loop (line 343) every 500ms, it introduces significant overhead and latency to the health probe.
Because the installed version on disk is guaranteed to be static once the maintenance phase begins, you should memoize this result to avoid repeated shell-outs during the polling cycle.
let cachedInstalledVersion: string | null | undefined;
function readInstalledPackageVersion(): string | null {
if (cachedInstalledVersion !== undefined) return cachedInstalledVersion;
const candidates: string[] = [
join(homedir(), '.bun', 'install', 'global', 'node_modules', '@automagik', 'genie', 'package.json'),
];
const npmPrefix = safeExec('npm prefix -g', 1500);
if (npmPrefix) {
candidates.push(join(npmPrefix, 'lib', 'node_modules', '@automagik', 'genie', 'package.json'));
}
for (const path of candidates) {
try {
const pkg = JSON.parse(readFileSync(path, 'utf-8')) as { version?: unknown };
if (typeof pkg.version === 'string' && /^\d+\.\d+/.test(pkg.version)) {
cachedInstalledVersion = pkg.version;
return cachedInstalledVersion;
}
} catch {
// try next candidate
}
}
cachedInstalledVersion = null;
return null;
}References
- It is acceptable to use hardcoded numeric limits (magic numbers) in non-critical fallback logic, especially when they serve as intentional caps to prevent performance issues like excessive I/O.
![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: 109c9b83e7
ℹ️ 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".
| const result = await runCommandSilent('pm2', ['jlist'], undefined, 3000); | ||
| if (!result.success) return null; |
There was a problem hiding this comment.
runPostUpdateMaintenanceSafe always calls restartServeIfStaleSafe, and this helper immediately executes pm2 jlist here. PM2 commands talk to a daemon and will launch one when none exists (per PM2 CLI/docs behavior around daemon startup), so on machines that merely have pm2 installed but do not supervise genie-serve, genie update can create an unintended background PM2 daemon and ~/.pm2 state as a side effect. Add a pre-check that PM2 is already active or that genie-serve is actually PM2-managed before calling pm2 jlist.
Useful? React with 👍 / 👎.
…le-serve # Conflicts: # src/genie-commands/__tests__/update.test.ts
1 participant
Summary
genie updatewas leaving the running pm2genie-serveprocess serving pre-update bytes from a deleted node_modules inode. The verify probe couldn't surface it becausereadServerHealthwas a tautology — it returned{ version: VERSION }from the calling CLI's compile-time constant rather than probing the daemon's actual state.Repro (today, on this host)
Fix
1. Truthful
readServerHealth— reads/proc/<pid>/cwd(Linux) to detect the kernel(deleted)marker AND reads the on-diskpackage.jsonfor the true installed version. Non-Linux platforms degrade gracefully.2. New
restartServeIfStale— runs inrunPostUpdateMaintenanceSafeBEFORE verify. Detects pm2 +genie-serve+ stale inode, issuespm2 restart genie-serve --update-env, polls for new pid. No-op when pm2 is missing, no genie-serve entry, or daemon already on live inode.3. New
daemon-stale-inodeVerifyResultvariant — banner carries pid, deleted-cwd path, and thepm2 restartremediation. Exits 1 alongsidehealth-unreachableso CI / scripted updaters notice instead of silently completing a half-update.Test plan
decideVerifyprecedence: inode-stale wins over version-mismatch (stronger signal)okregression lock (no false positives once restart succeeds)formatVerifyBannersurfaces pid,(deleted)cwd, andpm2 restart genie-serve/procprobe, pm2jlistparsinggenie update --nextonce@next 4.260506.4is published with this fix and confirm/proc/<new-pid>/cwdno longer carries(deleted)after the flow completes🤖 Generated with Claude Code