feat(kap-server): expose permanent session deletion via :delete - #3544
feat(kap-server): expose permanent session deletion via :delete#3544kimi-agent-bot wants to merge 10 commits into
Conversation
Add a delete session action to the v1 session-action route that
permanently removes a session through ISessionManager.delete, returning
{ deleted: true } and 40401 for unknown sessions. Publish a new
event.session.deleted Event2 from the session lifecycle controller and
fan it out to global WS subscribers with the same wire shape as
event.session.archived. Project the :delete operation into the OpenAPI
document alongside :archive.
Co-authored-by: qer <wbxl2000@outlook.com>
🦋 Changeset detectedLatest commit: a042545 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d60498642
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The broadcaster kept a deleted session's SessionState cached, so a later subscription or cursor request for that id could replay the retained event journal after a supposedly permanent deletion. The deletion branch now awaits any in-flight state creation, evicts and disposes the cached state (closing its journal), drops the transcript live store, and removes the journal file before fanning out event.session.deleted. Co-authored-by: qer <wbxl2000@outlook.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04e8ae1e45
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ails A failing journal rm (EACCES, EBUSY, an unexpected directory at the journal path) rejected the deletion branch before dispatchGlobal ran, so connected clients never learned that the session was permanently deleted even though the core deletion and REST response had already succeeded. The purge now logs cleanup failures and the event is dispatched regardless. Co-authored-by: qer <wbxl2000@outlook.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f35f8bb2c5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f35f8bb2c5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The global-events WebSocket consumer handled event.session.archived but ignored the new deletion event, so a session deleted by another client stayed visible in Kimi Inspect until the next polling tick. The switch now maps the payload session id to a new onSessionDeleted handler that removes the session from the activity store and invalidates the list, mirroring the archived path. Co-authored-by: qer <wbxl2000@outlook.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3a10a8c83
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Deleting a session removed the core session and its session-index entry but left its prompt and assistant text in the search MiniDb until a later writer sync happened to re-scan, so a search right after a permanent deletion could still return the deleted content. The delete route now purges the session's documents immediately through a new IGlobalSearchService.deleteSession, wired through the inline backend and the search worker protocol, with failures logged rather than failing the delete. Also classify the changeset as minor (new user-visible capability) and drop a redundant undefined union from the kimi-inspect callback type. Co-authored-by: qer <wbxl2000@outlook.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de53f98a4e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ledger The immediate per-document purge left three holes: it was not serialized against an in-flight sync that had snapshotted the session (the stale sync could recreate documents after the purge), it silently no-oped when another process held the MiniDb write lock, and a failed purge was logged and forgotten with no retry. Deletion now appends the session id to a bounded ledger file in the index directory first — durable even when the database itself is read-only — and every sync pass in any process purges the ledger's sessions idempotently, while search results filter ledger members outright so deleted content is never served in the meantime. The service also serializes the purge behind the in-flight sync and schedules a fresh pass, so the best-effort immediate deletion and the guaranteed ledger cleanup reinforce each other. Co-authored-by: qer <wbxl2000@outlook.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b628094846
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ts failure The read-modify-write ledger raced with itself: two concurrent deletions (or two read-only server processes) could read the same snapshot and overwrite each other's tombstone, and the 500-entry cap could evict intents the writer had not consumed yet. The ledger is now an append-only JSONL file, so concurrent records never clobber each other and no entry is ever evicted; every sync pass keeps purging the recorded sessions idempotently. When even the ledger append fails (disk full, unwritable directory), the error now propagates through the service instead of being logged and swallowed, so the delete route no longer reports success while nothing durable records the purge intent. Co-authored-by: qer <wbxl2000@outlook.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64310f8aff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
A transient ledger read failure (EACCES, EMFILE, I/O) was folded into an empty ledger, silently disabling the query-time filter and letting permanently deleted text surface; only ENOENT means empty now, search fails closed with index_unavailable when the ledger cannot be read, and the sync pass just skips its consumption round on a read error. And because the route now purges even when the session is already absent (still returning SESSION_NOT_FOUND to the caller), a deletion whose ledger write failed after the core session was removed stays retriable: retrying reaches the cleanup instead of dead-ending on the first line. Co-authored-by: qer <wbxl2000@outlook.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84375a8875
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ation-aware The broadcaster's journal rm was caught and only logged, so a supposedly permanent deletion could leave the per-session event JSONL on disk forever; removal now retries with bounded backoff and escalates to an error log when it keeps failing. Ledger lines now carry the deletion timestamp, and a sync pass that meets a newer incarnation of the same id retracts the tombstone instead of hiding the recreated session forever; passes also compact the ledger — retaining only entries appended mid-pass and tombstones whose session is still in the authoritative list (so a stale snapshot re-indexing it stays filtered) — keeping per-query ledger reads and per-pass purge work bounded over the installation's lifetime. Co-authored-by: qer <wbxl2000@outlook.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c24522b88
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…rnal retries The truncating ledger rewrite could overwrite a tombstone appended by another process mid-compaction, and dropping an acknowledged entry let a read-only peer serve the deleted content from its still-stale MiniDb view. The append log is now never rewritten: the writer alone periodically snapshots the effective tombstones (24h horizon plus entries still present in the authoritative list) into a sidecar file stamped with the append log's byte watermark, and every reader folds the snapshot with just the tail beyond that watermark — so a concurrent append is always included, readers behind a refresh keep filtering within the horizon, and per-query work stays bounded by the snapshot plus the unconsumed tail. Journal removal retries are now bound to the deleted incarnation: the retry tracks the journal's header epoch and cancels instead of deleting when the file at the path belongs to a recreated session with the same id. Co-authored-by: qer <wbxl2000@outlook.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a04254558a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .map(([id, at]) => [id, at]); | ||
| await writeFile( | ||
| this.deletedLedgerSnapshotPath, | ||
| JSON.stringify({ watermark: stats?.size ?? 0, entries }), |
There was a problem hiding this comment.
When a read-only server appends a deletion after readDeletedLedger() finishes but before this second stat(), stats.size includes bytes that are absent from the in-memory ledger. The snapshot then advances its watermark past that new line while omitting its session ID, so subsequent readers skip the tombstone and a stale sync can reindex or return the deleted transcript. Fresh evidence after the earlier compaction fix is that the watermark is measured after, rather than during, the ledger read; snapshot the exact boundary that was folded or coordinate snapshotting with appends.
AGENTS.md reference: packages/kap-server/AGENTS.md:L38-L40
Useful? React with 👍 / 👎.
| const entries = [...ledger] | ||
| .filter(([id, at]) => keepIds.has(id) || at >= horizon) | ||
| .map(([id, at]) => [id, at]); |
There was a problem hiding this comment.
When a session ID is recreated within 24 hours, the sync appends -<id> and adds the ID to retractedIds, but this filter still retains the original tombstone because its timestamp is newer than horizon. Since the snapshot watermark already covers the newly appended retraction, the next read restores the tombstone from entries and skips the retraction line, leaving the recreated session hidden from search and repeating this cycle on later syncs. Fresh evidence after the earlier recreation fix is this age clause retaining IDs explicitly marked as retracted; remove those IDs from the snapshot map regardless of age.
Useful? React with 👍 / 👎.
| const horizon = Date.now() - DELETED_LEDGER_HORIZON_MS; | ||
| const entries = [...ledger] | ||
| .filter(([id, at]) => keepIds.has(id) || at >= horizon) | ||
| .map(([id, at]) => [id, at]); |
There was a problem hiding this comment.
When a read-only process keeps its published MiniDb handle for more than 24 hours, this expiry removes an absent session's tombstone even though the snapshot watermark prevents the raw deletion line from being read again. That process's first later search detects the changed WAL but starts refresh() asynchronously while serving its stale handle, so without the tombstone it can return permanently deleted transcript text. Fresh evidence after the earlier stale-reader fix is the unconditional time-based expiry; retain the tombstone until readers cannot reference the pre-purge generation, rather than assuming 24 hours guarantees convergence.
AGENTS.md reference: packages/kap-server/AGENTS.md:L38-L42
Useful? React with 👍 / 👎.
| this.sessions.delete(payload.sessionId); | ||
| await disposeSessionState(state); | ||
| } | ||
| this.opts.transcriptService?.dropSession(payload.sessionId); | ||
| await this.removeSessionJournal(payload.sessionId); |
There was a problem hiding this comment.
When an explicit session ID is recreated before this detached deletion cleanup finishes, the preceding awaits allow this.sessions and the transcript store at that ID to belong to the new incarnation; these ID-only operations can then dispose the new state, drop its live transcript, and remove its journal. Fresh evidence after the earlier retry fix is that the epoch comparison protects only a failed retry—the initial successful rm and state cleanup are unguarded. Capture the deleted incarnation before yielding and verify it before every destructive operation.
Useful? React with 👍 / 👎.
| await writeFile( | ||
| this.deletedLedgerSnapshotPath, | ||
| JSON.stringify({ watermark: stats?.size ?? 0, entries }), | ||
| 'utf8', | ||
| ); |
There was a problem hiding this comment.
When another process searches while the writer executes this in-place writeFile, it can observe the snapshot after truncation but before the complete JSON is written and fail with index_unavailable; a writer crash in that window leaves malformed JSON permanently, and later syncs catch the read failure, replace the ledger with an empty map, and never repair the snapshot. Write a complete temporary file and atomically rename it so read-only processes always see either the old or new valid snapshot.
AGENTS.md reference: packages/kap-server/AGENTS.md:L38-L40
Useful? React with 👍 / 👎.
1 participant
Fixes #3543.
What
POST /api/v1/sessions/{id}:deleterouted to the existing v2ISessionManager.delete(real deletion; unknown ids surface the manager'sSESSION_NOT_FOUND/ 40401). Response schema{ deleted: true }, OpenAPI projection included.SessionDeletedevent (event.session.deleted, payload{ sessionId, workspaceId }) published fromSessionLifecycleService.delete— the controller level, which already injects the event service and owns the workspaceId, exactly mirroring howSessionArchivedis published fromarchive(). Both live and cold delete paths throughISessionManager.deleteare covered.workspace_id). The kimi-code-app client already maps and reducesevent.session.deleted, so other connected clients see deletions in real time.Verification
GET /sessionsimmediately.Note for reviewers
After a delete, the kap-server's own event journal
~/.kimi-code/server/events/<sid>.jsonlremains on disk (the session data itself is fully removed). Whether the journal should be cleaned up too is left as a follow-up question, not handled here.Co-authored-by: qer wbxl2000@outlook.com