fix(cli): harden import-jsonl + use curl installer for iii-engine upgrade#174
Merged
fix(cli): harden import-jsonl + use curl installer for iii-engine upgrade#174
Conversation
…rade ## import-jsonl Two failure modes surfaced as "Unexpected end of JSON input": - Livez probe only caught fetch throws, not non-OK responses. A stray service on port 3111 would pass the probe silently, then POST would 404 with an empty body and crash on res.json(). - res.json() is unforgiving: an empty body, HTML error page, or proxy response exits with a cryptic parse error instead of the actual HTTP status. Now: - Probe checks res.ok before continuing, with a start-the-server hint. - Import reads body as text, parses only if non-empty, and surfaces HTTP 401 (secret mismatch) and 404 (old server predating v0.8.13) with actionable guidance. ## upgrade cargo install iii-engine fails because iii-engine isn't on crates.io, and requireSuccess exited the whole upgrade before Docker ever ran. Switch to the official installer used everywhere else in the docs and demo command: curl -fsSL https://install.iii.dev/iii/main/install.sh | sh Installer failure is treated as optional — Docker pull still runs and the final note points at the Docker + release-binary fallbacks.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 Walkthrough🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli.ts`:
- Around line 815-828: The current probe block swallows the HTTP response
details and exceptions; update the probe logic around the fetch to preserve and
surface the failure reason: when calling fetch(`${base}/agentmemory/livez`) (the
probe variable), capture probe.status and a short probe body (e.g., await
probe.text() or first 200 chars) when probe.ok is false and include those
details in the p.log.error message (instead of treating all non-2xx as “not
running”); also catch and include the caught exception message when fetch throws
so the final error printed (before process.exit(1)) clearly distinguishes
“server unreachable” (network/error message) from “reachable but unhealthy”
(HTTP status and response body) while keeping probeOk boolean semantics.
- Around line 849-885: The response handling treats missing success as success
because json defaults to {}, so change the success check to require json.success
=== true (i.e., replace the current if (!res.ok || json.success === false) with
a condition that treats json.success !== true as failure). Ensure the code path
that logs failure (using spinner.stop("failed") and p.log.error with the
computed detail) also runs when json.success is absent/non-true, and only run
the final spinner.stop(success message) and use
json.imported/json.observations/json.sessionIds when json.success === true.
- Around line 771-774: The fallback warning uses a broken releases URL; update
the message string passed to p.log.warn (the branch guarded by installerOk
false) to replace "https://github.com/iii-dev/iii-engine/releases" with the
correct repository URL "https://github.com/iii-hq/iii/releases/latest" so the
warning points to the working releases page (locate the p.log.warn call near the
installerOk check).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+815
to
828
| let probeOk = false; | ||
| try { | ||
| await fetch(`${base}/agentmemory/livez`, { | ||
| const probe = await fetch(`${base}/agentmemory/livez`, { | ||
| signal: AbortSignal.timeout(2000), | ||
| }); | ||
| probeOk = probe.ok; | ||
| } catch { | ||
| p.log.error(`agentmemory is not running on port ${port}. Start it first.`); | ||
| probeOk = false; | ||
| } | ||
| if (!probeOk) { | ||
| p.log.error( | ||
| `agentmemory is not running on port ${port}. Start it with \`npx @agentmemory/agentmemory\` in another terminal, then re-run this command.`, | ||
| ); | ||
| process.exit(1); |
There was a problem hiding this comment.
Preserve the probe failure reason.
Line 820 treats every non-2xx /livez response as “not running”, but the livez endpoint can return 503 when the server is reachable but unhealthy. Include the HTTP status/body in the error so users can distinguish “down” from “running but not healthy.”
🛠️ Suggested adjustment
- let probeOk = false;
+ let probeOk = false;
+ let probeFailure = "";
try {
const probe = await fetch(`${base}/agentmemory/livez`, {
signal: AbortSignal.timeout(2000),
});
probeOk = probe.ok;
- } catch {
+ if (!probe.ok) {
+ const probeBody = await probe.text().catch(() => "");
+ probeFailure = `HTTP ${probe.status} ${probe.statusText}${probeBody ? ` — ${probeBody.slice(0, 160)}` : ""}`;
+ }
+ } catch (err) {
probeOk = false;
+ probeFailure = err instanceof Error ? err.message : String(err);
}
if (!probeOk) {
p.log.error(
- `agentmemory is not running on port ${port}. Start it with \`npx `@agentmemory/agentmemory`\` in another terminal, then re-run this command.`,
+ `agentmemory is not healthy on port ${port}${probeFailure ? ` (${probeFailure})` : ""}. Start it with \`npx `@agentmemory/agentmemory`\` in another terminal, then re-run this command.`,
);📝 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.
Suggested change
| let probeOk = false; | |
| try { | |
| await fetch(`${base}/agentmemory/livez`, { | |
| const probe = await fetch(`${base}/agentmemory/livez`, { | |
| signal: AbortSignal.timeout(2000), | |
| }); | |
| probeOk = probe.ok; | |
| } catch { | |
| p.log.error(`agentmemory is not running on port ${port}. Start it first.`); | |
| probeOk = false; | |
| } | |
| if (!probeOk) { | |
| p.log.error( | |
| `agentmemory is not running on port ${port}. Start it with \`npx @agentmemory/agentmemory\` in another terminal, then re-run this command.`, | |
| ); | |
| process.exit(1); | |
| let probeOk = false; | |
| let probeFailure = ""; | |
| try { | |
| const probe = await fetch(`${base}/agentmemory/livez`, { | |
| signal: AbortSignal.timeout(2000), | |
| }); | |
| probeOk = probe.ok; | |
| if (!probe.ok) { | |
| const probeBody = await probe.text().catch(() => ""); | |
| probeFailure = `HTTP ${probe.status} ${probe.statusText}${probeBody ? ` — ${probeBody.slice(0, 160)}` : ""}`; | |
| } | |
| } catch (err) { | |
| probeOk = false; | |
| probeFailure = err instanceof Error ? err.message : String(err); | |
| } | |
| if (!probeOk) { | |
| p.log.error( | |
| `agentmemory is not healthy on port ${port}${probeFailure ? ` (${probeFailure})` : ""}. Start it with \`npx `@agentmemory/agentmemory`\` in another terminal, then re-run this command.`, | |
| ); | |
| process.exit(1); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli.ts` around lines 815 - 828, The current probe block swallows the HTTP
response details and exceptions; update the probe logic around the fetch to
preserve and surface the failure reason: when calling
fetch(`${base}/agentmemory/livez`) (the probe variable), capture probe.status
and a short probe body (e.g., await probe.text() or first 200 chars) when
probe.ok is false and include those details in the p.log.error message (instead
of treating all non-2xx as “not running”); also catch and include the caught
exception message when fetch throws so the final error printed (before
process.exit(1)) clearly distinguishes “server unreachable” (network/error
message) from “reachable but unhealthy” (HTTP status and response body) while
keeping probeOk boolean semantics.
…ct releases URL Addresses review findings on #174: - Livez probe now captures probe.status + body snippet on non-OK responses and the exception message on fetch throws, then prints the distinct reason: "unreachable (...)" vs "reachable but unhealthy (HTTP 503: ...)". No more flat "not running" for every failure mode. - Import success check now requires json.success === true instead of treating a missing success field as success. A 2xx empty body used to fall through and print "imported 0 file(s)" — it now reports "response missing success field" and exits non-zero. - Fallback releases URL pointed at iii-dev/iii-engine, which doesn't exist. Corrected to iii-hq/iii/releases/latest — the same URL used in README and the demo command.
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.
Summary
Two hard-fails reported from a fresh `npx @agentmemory/agentmemory` session:
Changes
`runImportJsonl` — src/cli.ts
`runUpgrade` — src/cli.ts
Replaced `cargo install iii-engine --force` with the same installer used in the README, demo command, and docs:
```sh
curl -fsSL https://install.iii.dev/iii/main/install.sh | sh
```
Installer marked optional — failure no longer kills the Docker pull that follows. Warn points at `iiidev/iii:latest` and GitHub releases as fallbacks.
Dropped the unused `cargoBin` lookup.
Test plan
Reported in-session by @rohitg00 testing v0.9.0.
Summary by CodeRabbit
New Features
Bug Fixes