Skip to content

Harden recall and capture reliability - #78

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/recall-reliability-followups
Sep 23, 2026
Merged

graphite-app[bot] merged 1 commit into
mainfrom
codex/recall-reliability-followups

Conversation

@ishaanxgupta

Copy link
Copy Markdown
Contributor

Keeps recall fast and useful while making automatic capture bounded, nonblocking, and retry-safe.

ishaanxgupta commented Aug 22, 2026 •

Copy link
Copy Markdown
Contributor Author

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge-queue - adds this PR to the back of the merge queue
  • fast-track - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vorflux vorflux Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Found one material terminal-capture reliability gap.


Review with Vorflux

Comment thread src/services/capture.ts
await captureSessionEnd(sessionID);
snapshots.delete(sessionID);
activeSessions.delete(sessionID);
if (await captureSessionEnd(sessionID)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 — Retry terminal capture before disposal completes. Retaining the failed batch in these in-memory maps only helps if another lifecycle event arrives. On server.instance.disposed, this is the final event: a transient failure or the new 3-second timeout makes captureSessionEnd() return false, this handler returns, and process exit discards the only retry state. The stable customId makes a bounded retry safe, so please retry pending terminal batches inside this disposal path (or persist them outside process memory).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

more of a fall open kind of thing, i think we should have this? what do you think @sreedharsreeram

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree the hook should fail open. My concern is narrower: retaining the batch in these maps only enables retry if another lifecycle event arrives, but server.instance.disposed is the terminal event and process exit discards the maps. A bounded retry within the existing shutdown deadline would still fail open while covering one transient timeout; after that it can return and allow shutdown. If accepting possible loss of the final batch is intentional, the current behavior is consistent with that tradeoff.

Dhravya
Dhravya previously approved these changes Sep 1, 2026

Dhravya commented Sep 1, 2026 •

Copy link
Copy Markdown
Member

Merge activity

Keeps recall fast and useful while making automatic capture bounded, nonblocking, and retry-safe.
@graphite-app
graphite-app Bot force-pushed the codex/human-memory-capture branch from 316695f to c5f31a5 Compare September 23, 2026 00:51
@graphite-app
graphite-app Bot force-pushed the codex/recall-reliability-followups branch from 2a8b3fa to d108824 Compare September 23, 2026 00:52
@graphite-app
graphite-app Bot changed the base branch from codex/human-memory-capture to main September 23, 2026 00:55
@graphite-app
graphite-app Bot dismissed Dhravya’s stale review September 23, 2026 00:55

The base branch was changed.

@graphite-app
graphite-app Bot merged commit d108824 into main Sep 23, 2026
3 of 4 checks passed
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