Replies: 5 comments
|
Thanks for the source-level write-up and the fork patch — I verified your mechanism claims against master Your fix direction, however, runs into the one design fact I flagged in #5843, and I want to lay it out precisely so the tradeoff is visible to maintainers: The exact-match identity is a deliberate anti-cross-wire invariant, not an implementation detail.
Your change makes Why that's bounded (and why your patch is probably still acceptable for the listing path):
What I'd add before this lands as the upstream fix:
Given PRs are disabled, your fork commit ( |
|
Thanks for the detailed review and the design background! That makes total sense. We have pushed an update to the fork branch
All tests (509 tests in affected packages) and pre-commit hooks (translation pairing, lint, whitespace) pass cleanly. Maintainers are welcome to pull directly from the branch! |
|
That's a genuinely important correction to my suggestion, and you're right — I owe the nuance: The distinction you drew is the load-bearing one: for That also sharpens the fix's documentation story: the honest statement isn't "same fix covers both call sites" but "the display-hint path may relax under documented best-effort semantics; the descriptor path must not, because the cost of a wrong read is identity leakage rather than a stale title." Your docblock updates plus the ambiguity regression test (cold-without-count serves cut A; a caller that knows cut B strictly rejects it) give maintainers both the boundary and the proof. 509 tests + pre-commit hooks clean is a solid landing state. This is now the reference implementation for the family — maintainers can pull |
|
{"body": "Confirming this from another user: I hit exactly this on 0.1.2-rc.1 — after every restart, my long-lived compacted sessions show the workspace folder name and a stale "22d" timestamp in the session list until I open them. Opening restores title and time, but it comes back after the next restart. The thumbs-up on this report is mine — happy to test a fix build if useful."} |
|
Thanks for confirming with the concrete repro — the "long-lived compacted sessions" detail is worth a beat, because it may be a second variant of the same family rather than the same instance. The thread so far (Oct1AtJoe's fork fix at Compacted sessions are a distinct flag — if your affected sessions are not seeded forks, the trigger is likely the same cold-list projection staleness but reached through compaction (higher
If it reproduces on plain compacted sessions too, that's a separate repro worth its own note here (the fix for the seeded path may not cover it — different short-circuit). Either way, if any of your sessions are seeded forks, Oct1AtJoe's branch is testable today and would appreciate a second tester. |
Uh oh!
There was an error while loading. Please reload this page.
Summary
After restarting DSH, forked/seeded sessions (
isSeeded: true) show the workspace directory name as their title in the session list. Opening the conversation hydrates the full log and restores the real title, but restarting reproduces the issue.Root Cause
packages/api/session-controller/src/list.ts,projectionsFor(header, undefined)explicitly bypassedsessionProjectionCachefor seeded sessions (header.isSeeded ? undefined : ...).probeSmallCold), so they cannot be inspected on startup without opening the session.SessionSummary.projectionsis leftundefined, causingdisplayTitleOf(title, cwd, id)to fall back toworkspaceTitleOf(cwd).packages/session/session-projection-cache/src/index.ts,cachedSnapshotandcachedPredecessorTitlestrictly requiredinheritedEventCount. However,SessionHeaderduring cold listing only carriesversion,id,createdAt,cwd, andisSeeded— notinheritedEventCount.Proposed Fix
We have prepared and verified a clean fix with tests passing (508 tests in affected packages):
inheritedEventCount: Inpackages/session/session-projection-cache/src/index.ts, makeinheritedEventCountoptional incachedSnapshotandcachedPredecessorTitle. When omitted during cold listing,identityMatchesmatches bycreatedAt,cwd, andisSeeded.packages/api/session-controller/src/list.ts, querysessionProjectionCachefor cold sessions without skippingisSeeded.session-cold.host.spec.tsandcache.spec.tsto cover seeded cold listing and cache retrieval.References
Since external Pull Requests are disabled on this repository, the patch is available in the fork below:
All reactions