Skip to content

fix(session): preserve the starred flag in AddSession - #4182

Open
kitadesign wants to merge 1 commit into
docker:mainfrom
kitadesign:fix/add-session-preserve-starred
Open

fix(session): preserve the starred flag in AddSession#4182
kitadesign wants to merge 1 commit into
docker:mainfrom
kitadesign:fix/add-session-preserve-starred

Conversation

@kitadesign

Copy link
Copy Markdown

What

SQLiteSessionStore.AddSession built its own INSERT INTO sessions (...) that omitted
the starred column, so the flag was silently dropped on insert. UpdateSession,
addSessionTx and PersistCompaction all set it.

The user-visible effect is on fork: copySessionMetadata copies Starred to the new
session, and ForkSession persists it through AddSession — so forking a starred
session produces an unstarred fork
. Reachable from POST /api/sessions/:id/fork and
from the TUI fork path. The in-memory store keeps the flag, so the two backends disagree.

Why this shape

AddSession's INSERT was otherwise byte-for-byte identical to addSessionTx, which
already inserts starred correctly. Rather than adding the missing column to a second
list that can drift again, AddSession now calls addSessionTx. Net -1 line.

This is not a regression: the column list has been missing starred since the starring
feature was added in 875b4dd6 (Jan 2026), and it survived both 345b8aeed
(refactor(session): collapse duplicated session marshalling/SQL) and 93b1673b1
(refactor(session): scan SQL columns into native Go types) untouched.

Test

TestAddSession_PreservesStarred fails on main (Should be true) and passes with the
change. ./pkg/session/..., ./pkg/server/..., ./pkg/acp/..., ./pkg/evaluation/...
and ./pkg/tui/... all pass; go vet is clean.

Context

Noticed while mapping the session schema for #4181 (pluggable persistent session stores).
Happy to fold this into that work instead if you would rather keep it together.

@kitadesign
kitadesign requested a review from a team as a code owner September 7, 2026 04:53
@aheritier aheritier added area/core Core agent runtime, session management kind/fix PR fixes a bug (maps to fix:). Use on PRs only. status/needs-signed-commits Some commits in the PR are signed with a valid SSH/GPG key labels Sep 7, 2026
@aheritier

Copy link
Copy Markdown
Collaborator

👋 Some commits in this PR are not signed and verified by GitHub. Please sign your commits with a GPG or SSH key registered in your GitHub account, then force-push.

Commits that are not verified: 63e8c0f

See GitHub's guide on signing commits for setup instructions. I've added status/needs-signed-commits; it will be removed automatically once every commit in this PR carries a valid GitHub-verified signature.

SQLiteSessionStore.AddSession built its own INSERT that omitted the
starred column, while UpdateSession, addSessionTx and PersistCompaction
all set it. Forking a starred session therefore dropped the star, since
copySessionMetadata copies it and ForkSession persists via AddSession.

The INSERT was otherwise identical to addSessionTx, so call that instead
of maintaining a second column list that can drift again.

Noticed while mapping the session schema for docker#4181.
@kitadesign
kitadesign force-pushed the fix/add-session-preserve-starred branch from 63e8c0f to fcab72f Compare September 7, 2026 05:26
@kitadesign

Copy link
Copy Markdown
Author

Thanks — sorry for the noise. The signature was in place, but I registered the SSH signing key on my account about two minutes after the check ran, so 63e8c0f was still showing as unverified at that moment.

I've since force-pushed (identical tree, re-signed): fcab72f8b now reports verified: true / reason: valid via the API. The status/needs-signed-commits label looks stale rather than accurate — happy to do anything else if the check needs re-triggering.

@aheritier aheritier removed the status/needs-signed-commits Some commits in the PR are signed with a valid SSH/GPG key label Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/core Core agent runtime, session management kind/fix PR fixes a bug (maps to fix:). Use on PRs only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants