feat(btw): retain native side sessions - #1096
Conversation
|
@code-yeongyu All visible checks are green and the branch is mergeable, but the base ruleset blocks merge for this external fork and this account cannot enable auto-merge or request reviewers via API. Could you review and merge with a merge commit when satisfied? The PR body includes deterministic tests and real isolated xterm.js QA evidence. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8e789a1e0
ℹ️ 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".
|
@codex review |
|
@code-yeongyu Update: both Codex P1 threads are fixed in cb493d7 and resolved; focused tests are 60/60, check/build pass, real xterm QA is 25/25 with zero side tools, and visible checks are green. The upstream ruleset still blocks this external account, so the remaining action is maintainer review and merge-commit merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb493d7a5a
ℹ️ 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".
|
@codex review |
|
@code-yeongyu Final-head update: all four Codex P1 threads are fixed and resolved through 8735d2b. The branch is current with main; focused tests are 60/60, static check and production build pass, real xterm QA is 25/25, GitGuardian is green, and no review thread remains open. This external account still cannot execute the required merge-commit merge; maintainer merge is the sole remaining delivery action. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8735d2bc82
ℹ️ 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".
|
@codex review |
|
@code-yeongyu Actual final-head update: 549f8ce resolves all six Codex P1/P2 findings, including provider-native tool stripping and active-leaf snapshots. Expanded focused suite is 63/63 across 11 files; static check/build and real xterm 25/25 pass; no review thread is open. Final Codex re-review is requested. Maintainer merge-commit merge remains the only permission-gated action. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 549f8cebd2
ℹ️ 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".
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d537d31108
ℹ️ 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".
|
@codex review |
|
@code-yeongyu Current authoritative head ac875f4 is up to date with main and resolves all ten Codex findings, including prepared compaction/keepalive policy enforcement and settle-before-snapshot ordering. There are zero open threads; focused tests are 65/65, static check/build and real xterm 27/27 pass. Final Codex re-review is requested; maintainer merge-commit merge remains the only permission-gated action. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac875f4ee8
ℹ️ 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".
|
@codex review |
|
@code-yeongyu Current head 5419584 is up to date with main and resolves all twelve Codex findings, including durable assistant-less New BTW sessions and typed command-context discovery. Zero threads are open; focused tests are 67/67, static check/build and real xterm 30/30 pass. Final Codex re-review is requested; maintainer merge-commit merge remains the only permission-gated action. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5419584988
ℹ️ 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".
|
@codex review Exact head |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Ready for merge with a merge commit at exact head |
|
@codex review Exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a99ca3546e
ℹ️ 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".
| sessionToolPolicy: { | ||
| version: 1, |
There was a problem hiding this comment.
Enforce the tool ban for Cursor CLI sides
When the active model is cursor-cli-oauth and the user has acknowledged force execution, this policy does not make the retained side tool-free: the provider ignores the empty Senpi tool set, reads forceExecution (which defaults to true in cursor-cli-oauth/settings.ts:47-54), and spawns cursor-agent with the resulting force policy in cursor-cli-oauth/stream.ts:466-526. The provider's own guardrail explicitly states that this mode executes Cursor's tools autonomously (cursor-cli-oauth/guardrails.ts:8-13), so a supposedly isolated BTW question can still modify files or run commands. Propagate the disabled-session policy into this provider and force a non-executing mode, or reject this model for retained sides.
Useful? React with 👍 / 👎.
|
Ready for merge with a merge commit at exact head |
|
@codex review Exact head |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Status superseded: final user QA is pending. Do not merge. PR #1096 is intentionally draft while branch/check updates continue; merge readiness will be requested again only after user QA approval. |
|
Merge hold: user final QA is pending. Do not merge or approve a merge yet. This PR is intentionally draft; branch/check updates will continue. A fresh merge request will be posted only after the user reports final QA approval. |
|
Final QA gate: do not merge this PR yet. It is intentionally Draft while the user performs final QA. Until the user explicitly approves that QA, work is limited to keeping the branch and checks current. After approval, the PR will be marked ready and a fresh merge-commit request will be posted. |
|
@codex review Draft QA update — do not merge yet. Exact head |
|
Final QA handoff — do not merge yet. Exact head |
…sions # Conflicts: # packages/coding-agent/src/changes.md
|
User final QA approved. Refreshed Draft head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62778e388e
ℹ️ 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".
| - Retained BTW picker switches now revalidate the selected session after asynchronous pre-switch | ||
| guards, preventing a replaced JSONL path from opening an unrelated session. |
There was a problem hiding this comment.
Move the BTW entries into Unreleased
These new BTW fixes and feature notes are appended beneath the already-released 2026.8.24 heading while the corresponding Unreleased sections remain empty. This rewrites an immutable published release record and prevents the next release tooling from collecting the changes; move the additions to the appropriate Unreleased subsections.
AGENTS.md reference: AGENTS.md:L101-L102
Useful? React with 👍 / 👎.
| const spaceIndex = text.indexOf(" "); | ||
| const commandName = spaceIndex === -1 ? text.slice(1) : text.slice(1, spaceIndex); | ||
| return this._extensionRunner.getCommand(commandName)?.name === "btw-close"; |
There was a problem hiding this comment.
Restrict the close bypass to the BTW builtin
When a user extension also registers a command named btw-close, collision resolution gives both commands distinct invocation names but preserves .name === "btw-close" on each, so this check admits the foreign command while replacement is pending. A session-start handler or user dispatching that extension's suffixed invocation during rebind can therefore run arbitrary command logic before the replacement callback restores the target leaf; verify the resolved command's source identity rather than its canonical name alone.
Useful? React with 👍 / 👎.
|
@code-yeongyu User final QA is approved and PR #1096 is Ready for merge at exact head |
…sions # Conflicts: # packages/coding-agent/src/core/extensions/changes.md
|
@codex review Latest-main refresh for exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce9a799222
ℹ️ 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".
| const switchResult = await ctx.switchSession(selected.choice.sessionPath, switchOptions); | ||
| if ( | ||
| switchResult.cancelled && | ||
| (await dependencies.sessionIdentity(ctx, selected.choice.sessionPath)) !== expectedSessionId |
There was a problem hiding this comment.
Stop using the invalidated context after switch cancellation
When a guarded switch is cancelled after teardown has begun—for example because the selected path is replaced during shutdown, runtime construction, or rebind—the runtime recovers by creating a new outgoing session and invalidates this command context. Passing the stale ctx to sessionIdentity() makes the default helper catch the stale-context error as an identity mismatch, then continue causes the next loadCatalog(ctx) to throw, so /btw fails instead of refreshing the picker. Handle post-teardown cancellation through a freshly rebound context, or return without reusing this one.
Useful? React with 👍 / 👎.
What changed
/btw <question>create a distinct numbered side session with a bounded hidden Mainsnapshot, inherited model/thinking state, and no active tools.
/btwandapp.btw.switch, listing Main, retained sides, and NewBTW.
Why
The previous overlay could not retain multiple independent conversations, use Senpi's native
transcript/scrollback, survive reload/navigation, or distinguish non-destructive return from
destructive close. Ctrl+/ is also commonly reserved by editors such as Zed, so the picker now
advertises Ctrl+7 and bare
/btwas durable fallbacks.This supersedes #1019. That PR is conflicting and retains the old single-overlay/history model
rather than implementing native retained side sessions.
Safety and lifecycle details
when created while viewing another side.
newSessionpolicy capability persists the tool ban without coupling the BTW extensionto core internals. It forces later MCP/tool-search activation back to an empty effective set,
including reload/resume, blocks Cursor exec registered-tool lookup, and strips provider-native
tools after every extension payload transform through the shared normal/compaction/keepalive
request boundary.
The leaf ID persists in side metadata and is inherited by siblings created from a side; a newest
message beyond 64k is truncated rather than dropping all context.
newSessionreturns, so an assistant-less New BTW remainsdurable. Typed command-context listing/inspection keeps discovery behind the extension API.
replacement; selectors, inputs, and full extension editors share the guard; native tool prompt
contributors honor the side's typed no-tools policy.
reused filesystem paths cannot attach stale sides.
the renderer's actual focused component, including OAuth/API-key login flows.
return/close verify the destination Main ID before switching or deleting.
expected session ID before switching or creating.
64 KiB custom-entry inspection instead of opening every full transcript.
tool-specific prompts honor the retained no-tools policy.
is suppressed under retained no-tools policy.
duplicate-command test proves distinct
:1/:2invocation ownership.newSession().warning from the replacement session context.
Verification
npm run check: passednpm run build: passednode scripts/check-pr-changelog.mjs --base 706341a6bc3000d3a40e6933d112f970e1fe217c:10/10 production paths covered, passed
/btwand Ctrl+7 opened the same picker/reloadTerminal reservation note
Chrome, like Zed, reserved Ctrl+/ and Ctrl+_ without emitting a terminal byte in browser QA.
Deterministic Kitty and legacy-sequence tests cover Ctrl+/, Ctrl+_, and Ctrl+7 when delivered.
The real Ctrl+7 and bare
/btwfallbacks were exercised end to end.Residual risk
Native transcript scrolling still depends on the enclosing terminal keeping scrollback enabled,
which is the same contract as other regular Senpi sessions.