Skip to content

feat(cli): allow AGENTMEMORY_IMPORT_TIMEOUT_MS override for import-jsonl#607

Open
sakaoka082 wants to merge 1 commit into
rohitg00:mainfrom
sakaoka082:pr/import-jsonl-configurable-timeout
Open

feat(cli): allow AGENTMEMORY_IMPORT_TIMEOUT_MS override for import-jsonl#607
sakaoka082 wants to merge 1 commit into
rohitg00:mainfrom
sakaoka082:pr/import-jsonl-configurable-timeout

Conversation

@sakaoka082
Copy link
Copy Markdown

@sakaoka082 sakaoka082 commented May 22, 2026

Summary

Makes the import-jsonl CLI command timeout configurable via the AGENTMEMORY_IMPORT_TIMEOUT_MS environment variable, replacing the hardcoded 120s ceiling.

Why

The current 120s timeout in import-jsonl is hardcoded and fails for large jsonl imports (real-world scenario: 100 files / 25→59 sessions in a single batch hits the wall at 120s and aborts mid-flight).

Letting operators set their own ceiling — without touching source — enables larger batch imports on slower disks/networks while keeping the safe default.

Test plan

  • Default behavior unchanged: when AGENTMEMORY_IMPORT_TIMEOUT_MS is unset, the original 120000ms ceiling applies
  • Override works: AGENTMEMORY_IMPORT_TIMEOUT_MS=300000 raises the ceiling to 5 minutes
  • Invalid values (non-numeric) fall back to the default safely
  • Rebased onto upstream/main directly

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • The import-jsonl operation timeout is now configurable via the AGENTMEMORY_IMPORT_TIMEOUT_MS environment variable, with a default of 120 seconds if not specified or invalid.

Review Change Stack

Large ~/.claude/projects trees (>200 files / ~250MB) routinely take
more than the previously-hardcoded 120s client-side AbortSignal.timeout,
which surfaces a misleading 'import timed out after 2 minutes' to the
user even though the server keeps ingesting in the background.

Allow operators to extend the client-side timeout via
AGENTMEMORY_IMPORT_TIMEOUT_MS without patching the binary. Default
stays 120000 for backward compatibility. Non-positive / non-integer
values fall back to the default with a visible warning.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 22, 2026

@sakaoka082 is attempting to deploy a commit to the rohitg00's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

📝 Walkthrough

Walkthrough

The import-jsonl CLI command now reads its request timeout from the AGENTMEMORY_IMPORT_TIMEOUT_MS environment variable instead of using a hardcoded 120-second value. The timeout is validated as a positive integer, with a warning-based fallback to 120000ms if invalid, then applied via AbortSignal.timeout().

Changes

Import Timeout Configuration

Layer / File(s) Summary
Configurable import timeout
src/cli.ts
runImportJsonl parses AGENTMEMORY_IMPORT_TIMEOUT_MS as a positive integer with validation logic, warns and falls back to 120000ms if invalid, and passes the computed timeout to the /agentmemory/replay/import-jsonl request's AbortSignal.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A timeout from sky, no longer in stone,
The config flows in, through env it is known,
With guards and with care, should the value go wrong,
We fall back to one-twenty, steady and strong. 🐰⏱️

🚥 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 accurately describes the main change: making the import-jsonl timeout configurable via an environment variable instead of hardcoded.
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 unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

Copy link
Copy Markdown
Contributor

@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.

Caution

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

⚠️ Outside diff range comments (1)
src/cli.ts (1)

2443-2443: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the error message to reflect the actual timeout.

The error message is hardcoded to "2 minutes" but the timeout is now configurable. When users override AGENTMEMORY_IMPORT_TIMEOUT_MS, this message will mislead them during troubleshooting.

📝 Proposed fix
     if (err instanceof Error && err.name === "TimeoutError") {
-      p.log.error("import timed out after 2 minutes");
+      p.log.error(`import timed out after ${importTimeoutMs / 1000}s`);
     } else {
🤖 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` at line 2443, The error message currently hardcodes "2 minutes"
in the p.log.error call; update it to report the actual configured timeout by
reading the AGENTMEMORY_IMPORT_TIMEOUT_MS value (or the variable that holds it)
and formatting it into a human-readable duration (e.g., seconds or minutes)
before logging — replace the static string in p.log.error with a message that
includes the computed timeout string so overrides of
AGENTMEMORY_IMPORT_TIMEOUT_MS are reflected.
🤖 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.

Outside diff comments:
In `@src/cli.ts`:
- Line 2443: The error message currently hardcodes "2 minutes" in the
p.log.error call; update it to report the actual configured timeout by reading
the AGENTMEMORY_IMPORT_TIMEOUT_MS value (or the variable that holds it) and
formatting it into a human-readable duration (e.g., seconds or minutes) before
logging — replace the static string in p.log.error with a message that includes
the computed timeout string so overrides of AGENTMEMORY_IMPORT_TIMEOUT_MS are
reflected.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1c4bf558-4754-417a-aae9-f92af8d624d9

📥 Commits

Reviewing files that changed from the base of the PR and between bc64107 and a6255c6.

📒 Files selected for processing (1)
  • src/cli.ts

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