Skip to content

release: v0.9.3 — DX patch (feature-flag visibility + doctor command)#198

Merged
rohitg00 merged 3 commits intomainfrom
release/v0.9.3
Apr 24, 2026
Merged

release: v0.9.3 — DX patch (feature-flag visibility + doctor command)#198
rohitg00 merged 3 commits intomainfrom
release/v0.9.3

Conversation

@rohitg00
Copy link
Copy Markdown
Owner

@rohitg00 rohitg00 commented Apr 24, 2026

Summary

Developer-experience patch. Every disabled feature flag is now visible in the viewer, the CLI, and REST error responses, so devs no longer hit empty tabs wondering whether the install is broken or just opt-in. Adds a doctor command that diagnoses the whole stack in one shot and a first-run hero in the viewer that points at the magical-moment demo command.

Changes

New surfaces

  • agentmemory doctor — 10 diagnostic checks with fix hints on every failure
  • GET /agentmemory/config/flags — structured flag/provider/embedding state
  • Viewer banner — compact collapsible row at top of every tab
  • Viewer first-run hero — demo command prompt when sessions empty
  • Viewer footer — agentmemory viewer · v{ver} · github · docs · report issue → with preset GitHub issue body
  • Enriched empty states on Actions, Memories, Lessons, Crystals tabs

Enriched

  • status command shows provider + embedding + per-flag tick/cross
  • AGENTMEMORY_URL env now honored (was documented but ignored)
  • REST "not enabled" errors now return { error, flag, enableHow, docsHref } matching viewer banner contract

Website

  • Install section promotes demo to step 2 (magical moment on the critical path)
  • Version auto-derived via gen-meta.mjs prebuild (no more stale version drift)

Version bumps across 8 files per release checklist:

  • package.json, plugin/.claude-plugin/plugin.json, src/version.ts
  • src/types.ts (ExportData union), src/functions/export-import.ts (supportedVersions)
  • test/export-import.test.ts, packages/mcp/package.json, CHANGELOG.md

Test plan

  • npm run build — clean build
  • npm test -- test/consistency.test.ts test/export-import.test.ts — 13/13 pass
  • agentmemory doctor — 10 checks render correctly, hints shown on failures
  • agentmemory status — provider + embedding + flag table renders
  • AGENTMEMORY_URL=http://localhost:3111 agentmemory status — env honored
  • curl /agentmemory/graph/stats with flag OFF — returns { error, flag, enableHow, docsHref }
  • Viewer dashboard (live) — compact banner, expandable, dismissable
  • Viewer footer — version fills, feedback link opens pre-filled GitHub issue
  • Manual: fresh-install first-run hero (no sessions in KV)
  • Manual: agent-memory.dev landing shows v0.9.3 after website redeploy

CHANGELOG

See CHANGELOG.md [0.9.3] section for the complete list.

Summary by CodeRabbit

  • New Features
    • Added agentmemory doctor diagnostics, new REST endpoint for feature-flag metadata, and viewer banners with collapsible, dismissible, per-flag persistence and tab filtering.
    • Status/footer now show version/provider/flag info and a prefilled issue-report link.
  • Bug Fixes
    • Fixed banner scroll/layout behavior; CLI health/status honor custom server URLs/ports.
  • Documentation
    • Install guide updated to promote the demo command.
  • Chores
    • Package/site version bumped; site metadata generation updated; export format/version updated.

Every disabled feature flag is now visible everywhere: viewer banner, CLI
status, doctor output, and REST error responses. New `agentmemory doctor`
command runs 10 diagnostic checks in one shot, each failure includes a
concrete fix hint. Viewer gains first-run hero pointing at `demo` command
and a footer with preset issue-report link.

Bumps across 8 files per release checklist:
  package.json, plugin/.claude-plugin/plugin.json, src/version.ts,
  src/types.ts, src/functions/export-import.ts, test/export-import.test.ts,
  packages/mcp/package.json, CHANGELOG.md

See CHANGELOG for full details.
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agentmemory Ready Ready Preview, Comment Apr 24, 2026 6:52pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Adds developer-experience features: CLI doctor, enhanced status, new REST /agentmemory/config/flags, viewer feature-flag banners/first-run guidance, export/version bumps to 0.9.3, and website metadata generation updates.

Changes

Cohort / File(s) Summary
Version Bumps
package.json, packages/mcp/package.json, plugin/.claude-plugin/plugin.json, src/version.ts, website/lib/generated-meta.json
Bumped package/plugin and exported VERSION to 0.9.3; updated generated website meta version/restEndpoints/timestamp.
CLI Enhancements
src/cli.ts
Added doctor command with multi-step health/config checks; refactored base-URL resolution to honor AGENTMEMORY_URL (port extraction), added apiFetch() and extended status to show provider/embedding and per-flag states.
API Endpoint & Error Contracts
src/triggers/api.ts
Registered GET /agentmemory/config/flags returning provider/embedding and structured per-flag metadata; unified "feature not enabled" 503 response body used by gated endpoints.
Export/Types/Test Updates
src/functions/export-import.ts, src/types.ts, test/export-import.test.ts
Added "0.9.3" to supported export versions and ExportData.version union; updated export-import test expectation.
Viewer UI
src/viewer/index.html
Added collapsible feature-flag banners (filter by tab, per-flag localStorage persistence), provider/embedding warnings, richer empty states and first-run hero, footer version/reporting link; fixed banner stacking/scrolling.
Website Install Copy
website/components/Install.tsx, CHANGELOG.md
Promoted demo to step 2, renamed header to "THREE COMMANDS", and added 0.9.3 changelog entry documenting doctor, config flags endpoint, banners, CLI changes, and metadata generation.

Sequence Diagram(s)

sequenceDiagram
    participant User as User/CLI
    participant CLI as CLI Runner
    participant Engine as Engine Service
    participant Server as Server/Viewer
    participant API as API (/agentmemory/config/flags)
    participant Graph as Graph API

    User->>CLI: agentmemory doctor
    CLI->>Engine: GET /health
    alt engine healthy
        Engine-->>CLI: 200
    else
        CLI->>CLI: record failure / exit 1
    end
    CLI->>Server: GET /health
    alt server healthy
        Server-->>CLI: 200
    else
        CLI->>CLI: record failure / exit 1
    end
    CLI->>API: GET /agentmemory/config/flags
    alt flags returned
        API-->>CLI: flags + provider/embedding
        CLI->>Graph: GET /agentmemory/graph/stats
        alt graph populated & features enabled
            Graph-->>CLI: nodeCount > 0
        else
            CLI->>CLI: record failure / exit 1
        end
    else
        CLI->>CLI: record failure / exit 1
    end
    CLI->>User: consolidated pass/fail checklist
Loading
sequenceDiagram
    participant Viewer as Browser Viewer
    participant API as API Server
    participant LocalStorage as localStorage

    Viewer->>API: GET /agentmemory/config/flags
    API-->>Viewer: flags array (affects, enableHow, docsHref, provider info)
    Viewer->>Viewer: determine current tab
    Viewer->>LocalStorage: read dismissed flags
    loop per relevant flag
        alt dismissed
            Viewer-->>Viewer: skip rendering
        else enabled
            Viewer-->>Viewer: render info banner (collapsed by default)
        else disabled
            Viewer-->>Viewer: render collapsible warning with enableHow/docsHref
        end
    end
    Viewer->>LocalStorage: persist dismiss/collapse state on user action
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 I sniffed the flags and found the trail,
A doctor's hop to check each tale,
Banners fold and helpers sing,
Versions bumped — a tidy spring! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main release changes: version bump to 0.9.3 with two key DX improvements (feature-flag visibility and doctor command).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch release/v0.9.3

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

- Extract apiFetch() helper to deduplicate fetch/json/catch pattern
- Drop redundant --port re-parse in getRestPort (env already set by outer block)
- Unify safeDesc/safeCode in viewer banner render (single escHtml)
- Extract formatChecks() + DoctorCheck type in runDoctor
- Remove numbered step comments from runDoctor (flow speaks for itself)

Zero behavior change.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/viewer/index.html (1)

889-916: ⚠️ Potential issue | 🟠 Major

Keep the new banner/footer inside the scrollable layout.

#flag-banners and #viewer-footer are siblings of the fixed-height .view panes while body still has overflow: hidden. Because Line 171 still sizes each view for the old chrome, the added banner/footer push the bottom of the active tab below the viewport and make the footer unreachable on normal screen sizes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/viewer/index.html` around lines 889 - 916, The flag banner
(`#flag-banners`) and footer (`#viewer-footer`) are outside the scrollable area
while .view panes are fixed-height and body has overflow:hidden, causing the
active view content to be pushed below the viewport; fix by placing the banner
and footer inside the scrollable layout or converting the page to a column flex
layout where the header/banner, a central scrollable container for the .view
panes (make that container flex:1 and overflow:auto), and the `#viewer-footer` are
children so the .view content sizes correctly (use flex and/or calc heights
rather than fixed heights) so the footer remains reachable and scrolls with view
content.
🧹 Nitpick comments (4)
src/triggers/api.ts (3)

124-124: Minor: inner provider shadows the outer registerApiTriggers parameter.

The registerApiTriggers signature on line 86 already has a provider?: ResilientProvider | { circuitState?: unknown } parameter. Declaring const provider = ... inside this handler shadows it. It's harmless here (the outer provider isn't used in this callback), but a rename like providerKind / llmProviderKind would prevent future foot-guns if someone later wants to surface the real circuit state in this response.

♻️ Suggested rename
-      const provider = env["ANTHROPIC_API_KEY"] || env["GEMINI_API_KEY"] || env["OPENROUTER_API_KEY"] || env["MINIMAX_API_KEY"] ? "llm" : "noop";
-      const embeddingProvider = env["OPENAI_API_KEY"] || env["VOYAGE_API_KEY"] || env["COHERE_API_KEY"] || env["OLLAMA_HOST"] ? "embeddings" : "none";
+      const providerKind = env["ANTHROPIC_API_KEY"] || env["GEMINI_API_KEY"] || env["OPENROUTER_API_KEY"] || env["MINIMAX_API_KEY"] ? "llm" : "noop";
+      const embeddingProviderKind = env["OPENAI_API_KEY"] || env["VOYAGE_API_KEY"] || env["COHERE_API_KEY"] || env["OLLAMA_HOST"] ? "embeddings" : "none";
@@
-          provider,
-          embeddingProvider,
+          provider: providerKind,
+          embeddingProvider: embeddingProviderKind,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/triggers/api.ts` at line 124, The handler declares a local const provider
that shadows the registerApiTriggers parameter named provider; rename the local
variable (e.g., to providerKind or llmProviderKind) used in the expression
`env["ANTHROPIC_API_KEY"] || ... ? "llm" : "noop"` and update any references to
that local variable in the same callback so the outer parameter `provider` (from
registerApiTriggers) is no longer shadowed.

1024-1030: Enriched error payloads — consider extracting to a helper.

The four enriched 404 payloads (api::graph-query, api::graph-stats, api::graph-extract, api::consolidate-pipeline) duplicate the same { error, flag, enableHow, docsHref } shape, and the graph variants repeat the exact same four strings verbatim. Easy to drift over time (e.g., if the docs URL changes, it must be updated in three places).

Consider a small factory — also makes it trivial to reuse when you later enrich the remaining unchanged "not enabled" responses (e.g., Claude bridge, Snapshots, Team memory on lines 972, 993, 1147, 1167, 1185, 1271, 1290, 1311) so the structured-payload contract is uniform across the API.

♻️ Suggested helper
function flagDisabledResponse(opts: {
  error: string;
  flag: string;
  enableHow: string;
  docsHref: string;
}): Response {
  return { status_code: 404, body: { ...opts } };
}

const graphDisabled = () => flagDisabledResponse({
  error: "Knowledge graph not enabled",
  flag: "GRAPH_EXTRACTION_ENABLED",
  enableHow: "Set GRAPH_EXTRACTION_ENABLED=true and restart. Requires an LLM provider key.",
  docsHref: "https://github.com/rohitg00/agentmemory#knowledge-graph",
});

Also applies to: 1050-1056, 1085-1091, 1112-1118

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/triggers/api.ts` around lines 1024 - 1030, Extract a small factory like
flagDisabledResponse(opts) that returns { status_code: 404, body: { ...opts } }
and replace the repeated enriched 404 payload objects in the graph endpoints
(the responses used by api::graph-query, api::graph-stats, api::graph-extract,
and api::consolidate-pipeline) with calls to that factory (e.g., graphDisabled()
that passes the four strings for error, flag, enableHow, docsHref); update the
four duplicated literal objects to use this helper and reuse it for other
similar "not enabled" responses noted in the comment to keep the structured
payload consistent.

1024-1030: Nit: status_code: 404 for a disabled-feature flag is semantically odd.

A 404 implies "resource does not exist", but the route does exist — it's just gated off by config. 501 Not Implemented or 503 Service Unavailable (with the same structured body) more accurately describes "this capability is turned off on this server", and the new flag / enableHow / docsHref fields already convey the remediation. The viewer/CLI parse the JSON body anyway, so changing the status wouldn't break them. Not blocking — raising it so the client-side banner logic isn't anchored to 404 forever.

Also applies to: 1050-1056, 1085-1091, 1112-1118

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/triggers/api.ts` around lines 1024 - 1030, Replace the HTTP status_code
for the "Knowledge graph not enabled" responses (the response objects that
include flag: "GRAPH_EXTRACTION_ENABLED", enableHow, docsHref, etc.) from 404 to
a more accurate status like 503 Service Unavailable (or 501 Not Implemented) to
indicate the feature is disabled by configuration rather than the route missing;
update all identical response objects (the other occurrences noted around the
same blocks) so the client-side banner logic isn't tied to a 404.
src/cli.ts (1)

494-559: Remove the numbered // 1., // 2. comments in runDoctor().

They only restate what each block already does, so they're adding noise instead of structure here.

As per coding guidelines, "Avoid code comments explaining WHAT — use clear naming instead".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli.ts` around lines 494 - 559, The numbered inline comments inside
runDoctor() (e.g., "// 1. Server reachable", "// 2. Health status", "// 3.
Viewer reachable", etc.) are redundant and should be removed; edit the runDoctor
function to delete those numbered comment lines around the
server/health/viewer/LLM/embedding/flags/graph checks so the code relies on
clear variable and function names (serverUp, health, viewerUp, hasLlm, hasEmbed,
checks) without the noisy "1./2./3." comments.
🤖 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 91-95: The doctor and status code are still hardcoding a localhost
viewer URL instead of using getBaseUrl(); update doctor() to probe the viewer
using getBaseUrl() (e.g., build the viewer probe URL from getBaseUrl() + the
viewer path the code expects) instead of `http://localhost:${port + 2}` and
update the status-printing logic (status() / printStatus()) to display the URL
returned by getBaseUrl() rather than a localhost-derived address; ensure you
reference getBaseUrl() as the single source of truth for the base viewer URL so
remote/reverse-proxied installs are handled correctly.
- Around line 507-560: runStatus() currently checks graph?.totalNodes but the
/agentmemory/graph/stats endpoint may return nodes or nodeCount; update the
graph population check to consider all returned fields (e.g., graph?.totalNodes
|| graph?.nodes || graph?.nodeCount) and coerce to a number before comparing > 0
so the "Knowledge graph populated" check (the graphHas calculation used when
pushing the "Knowledge graph populated" check) correctly reflects existing data.

In `@src/triggers/api.ts`:
- Around line 121-187: The config-flags endpoint (api::config-flags) is
currently unauthenticated and leaks sensitive metadata; fix by enforcing the
same auth as other endpoints: add the middleware "middleware::api-auth" to the
sdk.registerTrigger config and/or call checkAuth(req,
process.env.AGENTMEMORY_SECRET) at the top of the api::config-flags handler
(before computing VERSION/provider/flags) so requests without a valid bearer
secret are rejected; reference the existing checkAuth helper, the
AGENTMEMORY_SECRET env var, and sdk.registerTrigger to mirror the
health/sessions endpoints' pattern.
- Around line 124-125: The embeddingProvider detection in the status logic
incorrectly omits OPENROUTER_API_KEY; update the expression that assigns
embeddingProvider (the variable in src/triggers/api.ts) to include
env["OPENROUTER_API_KEY"] alongside OPENAI_API_KEY, VOYAGE_API_KEY,
COHERE_API_KEY, and OLLAMA_HOST so it returns "embeddings" when only OpenRouter
is configured; ensure this matches the behavior of detectEmbeddingProvider() in
src/config.ts.

In `@src/viewer/index.html`:
- Around line 591-599: The clickable .flag-summary currently uses a div which is
not keyboard-focusable—replace the interactive div with a real <button
class="flag-summary" type="button"> (and do the same at the other occurrence
referenced) so it is natively focusable and operable by keyboard; keep the
existing .flag-summary CSS (add appearance: none; border: none; background: none
if needed to preserve visual styles), update any JS that reads/writes the
expanded state to use the button's aria-expanded attribute (e.g., toggle
aria-expanded="true"/"false") and ensure existing click handlers remain attached
to the button element.
- Around line 589-650: The new banner/card styles (.flag-summary, .flag-banner)
use var(--bg-subtle) which is not defined; add a --bg-subtle CSS custom property
to both theme blocks (light and dark) where other tokens live (e.g., alongside
--bg, --bg-alt, --border) so the background declarations don't fall back to
transparent; choose a subtle background color consistent with each theme (light:
a slightly lighter tint than --bg, dark: a slightly darker tint than --bg) and
place the definitions in the same theme selector so .flag-summary and
.flag-banner inherit them.

---

Outside diff comments:
In `@src/viewer/index.html`:
- Around line 889-916: The flag banner (`#flag-banners`) and footer
(`#viewer-footer`) are outside the scrollable area while .view panes are
fixed-height and body has overflow:hidden, causing the active view content to be
pushed below the viewport; fix by placing the banner and footer inside the
scrollable layout or converting the page to a column flex layout where the
header/banner, a central scrollable container for the .view panes (make that
container flex:1 and overflow:auto), and the `#viewer-footer` are children so the
.view content sizes correctly (use flex and/or calc heights rather than fixed
heights) so the footer remains reachable and scrolls with view content.

---

Nitpick comments:
In `@src/cli.ts`:
- Around line 494-559: The numbered inline comments inside runDoctor() (e.g.,
"// 1. Server reachable", "// 2. Health status", "// 3. Viewer reachable", etc.)
are redundant and should be removed; edit the runDoctor function to delete those
numbered comment lines around the server/health/viewer/LLM/embedding/flags/graph
checks so the code relies on clear variable and function names (serverUp,
health, viewerUp, hasLlm, hasEmbed, checks) without the noisy "1./2./3."
comments.

In `@src/triggers/api.ts`:
- Line 124: The handler declares a local const provider that shadows the
registerApiTriggers parameter named provider; rename the local variable (e.g.,
to providerKind or llmProviderKind) used in the expression
`env["ANTHROPIC_API_KEY"] || ... ? "llm" : "noop"` and update any references to
that local variable in the same callback so the outer parameter `provider` (from
registerApiTriggers) is no longer shadowed.
- Around line 1024-1030: Extract a small factory like flagDisabledResponse(opts)
that returns { status_code: 404, body: { ...opts } } and replace the repeated
enriched 404 payload objects in the graph endpoints (the responses used by
api::graph-query, api::graph-stats, api::graph-extract, and
api::consolidate-pipeline) with calls to that factory (e.g., graphDisabled()
that passes the four strings for error, flag, enableHow, docsHref); update the
four duplicated literal objects to use this helper and reuse it for other
similar "not enabled" responses noted in the comment to keep the structured
payload consistent.
- Around line 1024-1030: Replace the HTTP status_code for the "Knowledge graph
not enabled" responses (the response objects that include flag:
"GRAPH_EXTRACTION_ENABLED", enableHow, docsHref, etc.) from 404 to a more
accurate status like 503 Service Unavailable (or 501 Not Implemented) to
indicate the feature is disabled by configuration rather than the route missing;
update all identical response objects (the other occurrences noted around the
same blocks) so the client-side banner logic isn't tied to a 404.
🪄 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: 2a63fe94-2d7f-4149-b79e-e9ec8beb9bf5

📥 Commits

Reviewing files that changed from the base of the PR and between 196cbd6 and 16a1fe5.

📒 Files selected for processing (13)
  • CHANGELOG.md
  • package.json
  • packages/mcp/package.json
  • plugin/.claude-plugin/plugin.json
  • src/cli.ts
  • src/functions/export-import.ts
  • src/triggers/api.ts
  • src/types.ts
  • src/version.ts
  • src/viewer/index.html
  • test/export-import.test.ts
  • website/components/Install.tsx
  • website/lib/generated-meta.json

Comment thread src/cli.ts
Comment thread src/cli.ts
Comment thread src/triggers/api.ts
Comment thread src/triggers/api.ts Outdated
Comment thread src/viewer/index.html
Comment thread src/viewer/index.html Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/cli.ts (1)

524-530: ⚠️ Potential issue | 🟠 Major

runDoctor regressions still present from earlier review.

Two issues from prior review rounds are still in this revision:

  1. Line 524 — Viewer probe is hardcoded to http://localhost:${viewerPort}. For any non-local AGENTMEMORY_URL (or a URL without an explicit port — getRestPort() falls back to 3111 because new URL("https://x").port === ""), this check will always fail and viewerPort itself is meaningless. Reuse a viewer URL derived from getBaseUrl().
  2. Line 530graphHas only inspects graph?.totalNodes, while runStatus (line 438) and the viewer read graph?.nodes/graph?.nodeCount. Doctor will report "Knowledge graph populated" as failing on healthy installs that have data.
🛠️ Suggested fix
-  const viewerUp = await fetch(`http://localhost:${viewerPort}`, { signal: AbortSignal.timeout(2000) })
+  const viewerUrl = new URL(`:${viewerPort}`, base).toString().replace(/\/$/, "");
+  const viewerUp = await fetch(viewerUrl, { signal: AbortSignal.timeout(2000) })
     .then((r) => r.ok)
     .catch(() => false);
@@
-  const graphHas = (graph?.totalNodes || 0) > 0;
+  const graphNodeCount = graph?.nodes ?? graph?.nodeCount ?? graph?.totalNodes ?? 0;
+  const graphHas = graphNodeCount > 0;

Also consider plumbing viewerUrl into the failure hint on line 541 instead of just Port ${viewerPort} not responding.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli.ts` around lines 524 - 530, The viewer probe incorrectly uses a
hardcoded localhost URL and viewerPort; change the probe to use the actual
viewer base URL from getBaseUrl() (or construct viewerUrl via
getBaseUrl/getRestPort logic) instead of `http://localhost:${viewerPort}`, and
update the failure hint to reference viewerUrl rather than just the port
(symbols: viewerPort, getBaseUrl, getRestPort, viewerUrl). Also fix graph
detection: replace the single check `graphHas = (graph?.totalNodes || 0) > 0`
with a more complete check that considers graph?.nodes and graph?.nodeCount (the
same criteria used by runStatus and the viewer) so `graphHas` accurately
reflects a populated knowledge graph (symbols: graphHas, graph?.nodes,
graph?.nodeCount, runStatus). Ensure these changes are applied where runDoctor
constructs viewer probe and graphHas.
🤖 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 404-411: The apiFetch function currently returns res.json() for
all HTTP responses which treats 4xx/5xx error payloads as successful results;
update apiFetch (the async function apiFetch<T = unknown>(base: string, path:
string, timeoutMs = 5000)) to check res.ok before returning the parsed body—if
res.ok is false, parse the JSON error body (or text) to capture details and then
return null (or propagate a structured error) so callers like
runStatus/runDoctor don't receive malformed success objects; ensure the logic
uses the same AbortSignal.timeout and still handles network exceptions in the
existing catch path.

---

Duplicate comments:
In `@src/cli.ts`:
- Around line 524-530: The viewer probe incorrectly uses a hardcoded localhost
URL and viewerPort; change the probe to use the actual viewer base URL from
getBaseUrl() (or construct viewerUrl via getBaseUrl/getRestPort logic) instead
of `http://localhost:${viewerPort}`, and update the failure hint to reference
viewerUrl rather than just the port (symbols: viewerPort, getBaseUrl,
getRestPort, viewerUrl). Also fix graph detection: replace the single check
`graphHas = (graph?.totalNodes || 0) > 0` with a more complete check that
considers graph?.nodes and graph?.nodeCount (the same criteria used by runStatus
and the viewer) so `graphHas` accurately reflects a populated knowledge graph
(symbols: graphHas, graph?.nodes, graph?.nodeCount, runStatus). Ensure these
changes are applied where runDoctor constructs viewer probe and graphHas.
🪄 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: b83a983d-d10c-4759-bca3-d40f44089566

📥 Commits

Reviewing files that changed from the base of the PR and between 16a1fe5 and f937c8b.

📒 Files selected for processing (2)
  • src/cli.ts
  • src/viewer/index.html
✅ Files skipped from review due to trivial changes (1)
  • src/viewer/index.html

Comment thread src/cli.ts
Comment on lines +404 to +411
async function apiFetch<T = unknown>(base: string, path: string, timeoutMs = 5000): Promise<T | null> {
try {
const res = await fetch(`${base}/agentmemory/${path}`, { signal: AbortSignal.timeout(timeoutMs) });
return (await res.json()) as T;
} catch {
return null;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

apiFetch ignores HTTP error responses.

apiFetch calls res.json() without first checking res.ok, so a 4xx/5xx response with a JSON error body (e.g., the new structured { error, flag, enableHow, docsHref } payload from "not enabled" routes) is returned as a successful result. This silently feeds malformed objects into runStatus/runDoctor (e.g., flags?.flags becomes undefined, health?.status is missing, doctor reports green/red incorrectly).

🛡️ Suggested fix
 async function apiFetch<T = unknown>(base: string, path: string, timeoutMs = 5000): Promise<T | null> {
   try {
     const res = await fetch(`${base}/agentmemory/${path}`, { signal: AbortSignal.timeout(timeoutMs) });
+    if (!res.ok) return null;
     return (await res.json()) as T;
   } catch {
     return null;
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli.ts` around lines 404 - 411, The apiFetch function currently returns
res.json() for all HTTP responses which treats 4xx/5xx error payloads as
successful results; update apiFetch (the async function apiFetch<T =
unknown>(base: string, path: string, timeoutMs = 5000)) to check res.ok before
returning the parsed body—if res.ok is false, parse the JSON error body (or
text) to capture details and then return null (or propagate a structured error)
so callers like runStatus/runDoctor don't receive malformed success objects;
ensure the logic uses the same AbortSignal.timeout and still handles network
exceptions in the existing catch path.

- cli: use getBaseUrl()-derived viewer URL instead of hardcoded localhost
  (new getViewerUrl() helper; honors AGENTMEMORY_VIEWER_URL + base hostname)
- cli: null-safe graph count (totalNodes || nodes || nodeCount) in status
  + doctor — matches viewer dashboard fallback
- api: auth config-flags via middleware::api-auth + explicit checkAuth
  (leaked provider + version state previously)
- api: use detectEmbeddingProvider() from config instead of hand-rolled
  key check (was missing GEMINI_API_KEY + OPENROUTER_API_KEY; now matches
  config.ts single-source-of-truth)
- api: rename local 'provider' to 'providerKind' to avoid shadowing
  registerApiTriggers() parameter
- api: extract flagDisabledResponse() factory + graphDisabledResponse()
  + consolidationDisabledResponse() helpers; status_code 404 -> 503
  (disabled-by-config != missing route)
- viewer: add --bg-subtle token to both light + dark themes
  (6 call sites previously fell back to transparent)
- viewer: .flag-summary div -> button with aria-expanded + aria-controls
  for keyboard a11y
- viewer: body flex column + .view flex:1 min-height:0 so banner + footer
  sit outside scroll region without clipping view content
- viewer: dashboard + graph tab prefer totalNodes/totalEdges (matches
  actual mem::graph-stats response; pre-existing fallback bug)

Zero behavior change in happy path. Fixes latent correctness + a11y issues.
@rohitg00 rohitg00 merged commit 18e3257 into main Apr 24, 2026
4 of 5 checks passed
@rohitg00 rohitg00 deleted the release/v0.9.3 branch April 24, 2026 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant