Conversation
… session `Session::save()` returns early when a session has no visible message, no title and no parent, on the grounds that a freshly opened panel must not turn its hidden session-context message into a transcript on disk. That guard already carries exemptions for caller-set explicit state (`custom_title`, `title`, `parent_id", added by 1jehuang#1144), but `is_debug`, `is_canary` and `improve_mode` are explicit state by the same argument and were never added, so setting one and saving drops it silently: `save()` returns `Ok(())`, nothing is logged, and a later load-by-id fails with ENOENT. The issue names three e2e failures and one `jcode-tui` failure; the `improve_mode` clause is required for the fourth, which the issue's suggested fix (is_debug and canary only) would have left failing. Reproduced on macOS at origin/master `2a4edaa02` with the guard reverted: e2e session_flow: 3 failed (all "No such file or directory (os error 2)") test_clear_preserves_debug_for_resumed_debug_session test_debug_create_selfdev_session_marks_canary test_debug_create_session_marks_debug jcode-tui --lib: 1 failed (same error) test_improve_mode_persists_in_session_file With this change: cargo test --test e2e session_flow:: -- --test-threads=1 test result: ok. 6 passed; 0 failed cargo test -p jcode-tui --lib test_improve_mode_persists_in_session_file test result: ok. 1 passed; 0 failed Two intermediate measurements, so each clause is shown necessary rather than merely stated: with only the is_debug/is_canary clauses the three e2e tests pass and `test_improve_mode_persists_in_session_file` still fails with the identical ENOENT, which is why the improve_mode clause is here. Related: 1jehuang#1119, 1jehuang#1249, 1jehuang#1144.
Owner
Author
|
Closing this fork-side validation PR because upstream 1jehuang#1373 was closed as superseded by current master coverage. No fork merge or branch deletion is needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fork-side validation of the change proposed upstream as 1jehuang#1373 for 1jehuang#1339.
A
pr/*branch has to be based onorigin/masterto keep the upstream PR scoped, but that means it carries upstream'sci.yml, whose trigger ispush: branches: [main, master]. Branch pushes to the fork therefore produce no CI run for this branch at all; the fork's ownpush: branches: ["**"]only applies to branches that contain it, i.e. branches based onfork/master. Opening this PR againstfork/masteris what puts the change through the fork's secretless CI and Greptile, becausepull_requestmatches and the workflow comes from the base.Do not merge this: the upstream PR is 1jehuang#1373, and a merge commit here would end up inside that PR's diff.
The code change appears safe, narrowly scoped, and consistent with the existing session persistence lifecycle.
Summary
The PR updates the new-session persistence guard so sessions containing debug, canary, or improve-mode state are written even before they have visible messages, titles, or parent linkage.
Reviews (1) · Last reviewed commit: "fix(session): persist debug, canary and ..."