fix(agent-core-v2): roll back failed session initialization - #1642
fix(agent-core-v2): roll back failed session initialization#1642wbxl2000 wants to merge 9 commits into
Conversation
🦋 Changeset detectedLatest commit: b7a9100 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
|
@codex review |
commit: |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a5dd5318d
ℹ️ 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: 833858caf7
ℹ️ 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
When fork() is requested while the source session's create/resume has registered its handle but has not published yet (for example, it is blocked in ensureMcpReady() or later fails), this raw sessions lookup bypasses the new initializations visibility gate and treats a half-initialized source as forkable. That can persist a fork of a source session that is subsequently rolled back or closed, violating the new invariant that initializing sessions stay hidden; wait for the source initialization to publish/settle or resolve it through get()/resume() before using the handle.
ℹ️ 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".
|
Addressed the source-visibility finding in @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d8c880dd8
ℹ️ 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 New HEAD: Scope note: this PR remains draft and must not ship independently. It still depends on task4-B for the shared cross-process/global session-ID lock, v1 filtering of final directories without valid state, safe final |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20e75aed95
ℹ️ 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 New HEAD: The published-handle rollback P2 is fixed with publication as the irreversible commit point; full |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75502ee8da
ℹ️ 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 New HEAD: The creation-hook fork deadlock is fixed by distinguishing unpublished, published, and closing source claims. Lifecycle is 64/64 and full |
…lize-rollback # Conflicts: # packages/agent-core-v2/src/app/sessionLifecycle/sessionLifecycleService.ts
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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 Final merge HEAD: The logical fix at |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Related Issue
No linked issue. This fixes Session scopes and on-disk state retained after initialization failures.
Problem
Session handles are registered before metadata, MCP, plan bootstrap, resume replay, and creation hooks finish. If any of those steps rejects, the handle and child DI scope can remain reachable. A fresh create can also leave
state.jsonand partial session files behind, so later list/resume/fork calls treat the failed attempt as a persisted session.Concurrent same-ID initialization and teardown previously had no single lifecycle owner. One attempt could expose a half-initialized handle, overwrite another handle, return a handle that a hook had already closed, or delete a replacement during rollback.
What changed
get,list, andresumehide unpublished or closing handles; creation hooks can safely call same-ID read APIs after core publication without waiting on themselves.mkdir. Only the call that created that leaf may remove it;EEXISTreportsSESSION_ALREADY_EXISTS, resume never claims or deletes an existing directory, and identity checks protect concurrent replacements.ISessionMetadata.whenIdle(), dispose the Session scope, and wait for append-log writes. A failure in those pre-removal barriers retains the directory; every cleanup error is reported together with the original failure throughAggregateError.FileSessionIndexnow requires matching persisted state andcreatedAtbefore a cached row can answergetor filteredlist.Required follow-up before release
This PR is a prerequisite for the task4-B session-membership change, not a complete cross-process publication transaction, and should not ship by itself.
Task4-B must still own these related contracts once, rather than duplicating them here:
list/getfiltering for final directories that do not contain a validstate.json;session_index.jsonlpublication only after all fallible initialization, or a durable invalidation record for ambiguous/failed publication;createdAtcomparison in this PR is only a temporary stale-generation guard, not an ownership token.Latest
main, including #1635, is merged into this branch, so cleanup now runs on the stronger append-log flush/retirement contract. A sticky failure in an unrelated append-log key still makes cleanup conservatively retain the directory and returnAggregateError; an owner-scoped barrier would be a separate availability improvement. The pre-existing fully-live fork-versus-close race after initialization ownership retires also remains outside this PR.Validation
agent-core-v2: 236 files / 3332 tests passed with one workerkap-server: 55 files / 615 tests passed with one worker; typecheck passedserver-e2e: v2 in-process smoke 5/5; typecheck passedagent-core-v2: build, typecheck, and domain lint passed (848 files)git diff --checkpassedThe first
kap-serverrun used an invalid test home shape and also hit the known auth temp-directoryENOTEMPTYcleanup flake. The two exact files passed 26/26 with<temp>/.kimi-code, then the corrected full run passed. Oneagent-core-v2full run hit the unchanged five-second full-compaction boundary test; the exact case passed 3/3, then the full suite passed. Changeset generation was re-checked; repository-widechangeset statusis currently blocked by an unrelated pre-existing changeset referencing a removed workspace.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.