Skip to content

feat(recall): make timeout configurable - #58

Merged
noctuid merged 1 commit into
mainfrom
configurable-timeout
Sep 14, 2026
Merged

noctuid merged 1 commit into
mainfrom
configurable-timeout

Conversation

@noctuid

@noctuid noctuid commented Sep 14, 2026

Copy link
Copy Markdown
Owner

Add recallTimeoutMs for auto-recall and the recall tool while preserving the existing 10-second default. Recall failures continue without retries or degraded fallback behavior.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Malformed timeout values with numeric prefixes are silently accepted instead of warning and falling back.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds configurable recall timeouts while retaining the 10-second default.

Changes:

  • Adds config-file and environment-variable support.
  • Applies the timeout to auto-recall and the recall tool.
  • Adds tests and documentation.
File summaries
File Description
src/config.ts Defines, loads, and validates the timeout.
src/client.ts Uses the configured recall timeout.
tests/client.test.ts Tests client timeout behavior.
tests/config.test.ts Tests configuration loading and validation.
tests/fixtures.ts Updates shared fixtures and environment cleanup.
tests/recall.test.ts Tests auto-recall timeout behavior.
tests/tools.test.ts Tests recall-tool timeout behavior.
README.md Adds configuration guidance.
docs/reference.md Documents the setting and environment variable.
CHANGELOG.md Records the feature under Pending.
AGENTFEEDBACK.md Records changelog placement guidance.
Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/config.ts Outdated
@noctuid
noctuid force-pushed the configurable-timeout branch from 05bb603 to d4e03d5 Compare September 14, 2026 22:26
@noctuid
noctuid requested a balanced review from Copilot September 14, 2026 22:26
@noctuid

noctuid commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Timeout values exceeding the JavaScript timer range are accepted but can cause immediate timeouts.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/config.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d4e03d546c

ℹ️ 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".

Comment thread src/config.ts Outdated
Add recallTimeoutMs for auto-recall and the recall tool while
preserving the existing 10-second default. Recall failures continue
without retries or degraded fallback behavior.

Fixes #52
@noctuid
noctuid force-pushed the configurable-timeout branch from d4e03d5 to 129363f Compare September 14, 2026 22:39
@noctuid
noctuid merged commit 12d4f6e into main Sep 14, 2026
2 checks passed
@noctuid
noctuid deleted the configurable-timeout branch September 14, 2026 22:40
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.

2 participants