Skip to content

Fork validation for #1373 (upstream #1339): persist debug, canary and improve-mode state - #4

Closed
ianalitis wants to merge 1 commit into
masterfrom
pr/session-persist-explicit-state
Closed

ianalitis wants to merge 1 commit into
masterfrom
pr/session-persist-explicit-state

Conversation

@ianalitis

@ianalitis ianalitis commented Sep 22, 2026 •

Copy link
Copy Markdown
Owner

Fork-side validation of the change proposed upstream as 1jehuang#1373 for 1jehuang#1339.

A pr/* branch has to be based on origin/master to keep the upstream PR scoped, but that means it carries upstream's ci.yml, whose trigger is push: branches: [main, master]. Branch pushes to the fork therefore produce no CI run for this branch at all; the fork's own push: branches: ["**"] only applies to branches that contain it, i.e. branches based on fork/master. Opening this PR against fork/master is what puts the change through the fork's secretless CI and Greptile, because pull_request matches 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.

RetriggerConfidence Score: 5/5

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.

  • Prevents flag-only saves from silently succeeding without creating a loadable session file.
  • Uses the existing snapshot and journal mechanisms, which already serialize and restore all three fields.
  • Existing improve-mode and headless-session tests exercise the intended persistence behavior.

Reviews (1) · Last reviewed commit: "fix(session): persist debug, canary and ..."

… 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.
@ianalitis

Copy link
Copy Markdown
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.

@ianalitis ianalitis closed this Sep 27, 2026
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.

1 participant