Skip to content

fix(client): never hand the shell a session id its list does not know (uncaught sessions.retain on load) - #2207

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/2-landing-part-unknown
Nov 2, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/2-landing-part-unknown

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2206

Owner: Dev-2
Written-by: Dev-2
Reviewer: Birfy
Claim-Id: 482822f357a145659d39f6b071fe98fe
Tier: T1
Base: 77a6b4d
Tree: 0c5be985b41d9f7f09ba434d5c0717ea91f008bd
Verified commit: ae42bd5

The defect

Loading the client threw two uncaught sessions.retain: unknown session <id> errors on the console (reported in #1306; here the session is the instance's own Main Bot part). No visible symptom — the page simply carried two errors.

The chain, on main:

  1. core/src/client/sidebar.js:686 runs actions.landOn(roster.mainBotId, …) from a 400 ms timer, with no click behind it and no .catch.
  2. landOn (core/src/client/index.js:452) already passes the session list into landingPart (index.js:454).
  3. landingPart (core/src/client/text.js:85) answered from the roster alone — the final return main ? currentPart(main) : mainId never consulted sessions.byId, while the held branch on the line above did.
  4. The shell's replaceMain calls sessions.retain(id, { source: 'mainView' }), which throws synchronously for an id it does not know (@deepseek-ai/dsh-api-session-controller 0.2.0-rc.2 lib/client.js:2385). From a timer, that throw is an uncaught error.

So FlyBot's own roster answer ran ahead of the shell's catalog: on a cold load the catalog is { byId: {}, phase: 'pending' } and the landing still nominated the Main Bot's part id.

Failing test first

core/test/land-on.test.js, one new test (#1306: a destination the session list does not know is never handed to the shell), written and run before any source change, on this branch's single parent 77a6b4d4 (the same test and source also red on c57efcfa when the work started):

test('#1306: a destination the session list does not know is never handed to the shell', () => {
  const pending = { byId: {}, phase: 'pending' }
  assert.equal(landingPart(roster, null, 'chief', { sessions: pending }), undefined, 'a session list that has not loaded: no destination')
  assert.equal(landingPart(roster, last, 'chief', { stay: true, sessions: pending }), undefined, 'the held chat is not in the list either')
  const fresh = { ...friend, sessionId: 'friend-2' }
  const gone = { byId: { 'chief-2': { id: 'chief-2' } } }
  assert.equal(landingPart({ byId: { friend: fresh, chief } }, last, 'chief', { stay: true, sessions: gone }), 'chief-2', 'a fresh part the list has not caught up with, the chat it replaced gone: the Main Bot')
  assert.equal(landingPart({ byId: { friend: fresh, chief } }, last, 'chief', { stay: true, sessions }), 'friend-1', 'the fresh part is unknown, the chat it replaced is known: stay on that chat')
})

Before the fix: # tests 7 / # pass 6 / # fail 1

not ok 7 - #1306: a destination the session list does not know is never handed to the shell
  error: |-
    a session list that has not loaded: no destination
    + actual - expected
    + 'chief-2'
    - undefined
  code: 'ERR_ASSERTION'
  actual: 'chief-2'
  operator: 'strictEqual'

The six existing tests were not touched, including land-on.test.js:33 landingPart({ byId: {} }, null, 'chief') === 'chief': that contract is "no catalog passed → id fallback", and landOn always passes the list, so it does not need to change.

The fix

core/src/client/text.js:85-102 — landingPart builds the candidates it already knew, in the same priority, then answers only with a destination the list knows when the caller passed one:

const destinations = [
  bot && currentPart(bot) !== last.sessionId ? currentPart(bot) : undefined,
  bot && stay ? last.sessionId : undefined,
  held ? last.sessionId : undefined,
  main ? currentPart(main) : mainId,
].filter(id => id !== undefined)
if (sessions === undefined) return destinations[0]
return destinations.find(id => sessions.byId?.[id] !== undefined)
  • A fresh part the list has not caught up with falls back to the chat it replaced, then to the Main Bot — the same order, now filtered by the list.
  • Nothing known at all → undefined.
  • No list passed (other callers) → the old id fallback, unchanged.

core/src/client/index.js:452-462 — landOn returns on undefined before any navigation:

const target = landingPart(…, sessions: ctx.sessions.list.getSnapshot() })
if (target === undefined) return noteNavigation('refused', id, last?.sessionId)
setPlain(false)
noteNavigation('landOn', target, last?.sessionId)

noteNavigation returns undefined, so this is a bare return to the caller (nothing is handed back, no 802): no uiWorkspace.openSession, no expected = target. setPlain(false) moved below the guard — on main it ran before landingPart, i.e. the surface was cleared for a navigation that could not happen; now a refusal leaves the view exactly as it is. That move is part of this fix, not a drive-by: clearing the surface and then throwing is the state the report's page ended in.

Deliberately not changed:

  • index.js:391 openSession (target from partFor): its callers are a click (row drawn from the same roster + list) or openSession(mainBotId) after archive/remove; an unknown id there throws inside a click handler, not the 400 ms timer, and "open a session the list does not have — create or refuse?" is a separate design question.
  • index.js:399 openAgentSession and :425 newAgentSession: their ids come from list rows, from the host's agent.sessions, or from a session the host just created — the client list snapshot can lag one beat behind, and a catalog test there would refuse a legitimate open.
  • sidebar.js:674/:686 get no .catch: the throw is gone at the source (it is a returned undefined now); swallowing exceptions at the timer would hide the next real defect. If a safety net is ever wanted, it belongs inside landOn (try/catch → quietly('landOn')), not twice at the call sites.

Verification

Run on the pushed tip ae42bd5b15c57ecb5445c69b74e5e15f27d89e3d (tree 0c5be985b41d9f7f09ba434d5c0717ea91f008bd), clean tree (git status --porcelain empty):

  • TZ=UTC ARCH_DOCS_BASE=77a6b4d4 npm run verify → exit 0: core 2665/2665/0 · .github/scripts 598/598 · social 555 (554 + 1 skip) · tree 127 (126 + 1) · timeline 140 (139 + 1) · workflow 368/368 · interop 37/37 · live-e2e 24/24 · arch-docs: OK (10 changed file(s)) · build-client --check silent · ds-bot-patches check three ok lines + canonical ok (generate is a no-op).
  • Run separately before the suite: node core/scripts/build-client.mjs --check → exit 0 (silent); node scripts/ds-bot-patches.mjs check → the three ok lines; node scripts/ds-bot-patches.mjs canonical → ok (generate is a no-op).
  • Same test after the fix: 7 tests / 7 pass / 0 fail.
  • Red-first numbers above come from a clean 77a6b4d4 worktree before the source change (7 tests / 6 pass / 1 fail); with the fix the same file is 7/7, and the tip suite is the 2665/2665 above.

Not run: the browser instance (no reproduction of the uncaught error in a real window — the mechanism is covered by the unit test, not by a page load), dev/bench.sh smoke (no index.js/src/host/ change), the CI package job's typecheck/actionlint/pack check, check-mojibake (no outside bytes were decoded: core/client.js and patches/ were regenerated by their own generators).

Generators: (cd core && npm run build) → node scripts/ds-bot-patches.mjs generate → check and canonical, all from this tree; the generated files in the diff are core/client.js, patches/ds-bot/0001-core-fixes.patch, 0003-flybot-api-client.patch, 0008-client-bundle.patch, patches/ds-bot/files/test/land-on.test.js.

Docs: docs/subsystems/core-client.md (the landing paragraph now says the session list decides whether an id may be handed to the shell, #2206). Changelog: changelog.d/2206-landing-part-unknown.fixed.md.

Residual risk

If a fresh part's session summary reaches the client later than the roster does, the view lands on the chat that part replaced (while that chat is still in the list) instead of the new part; nothing throws and no page is lost, and the next roster/list change re-runs the effect. No retry subscription on the session list was added — that would widen the change beyond the reported defect.

Checkpoint

{
"pr": 2207,
"claim": "Dev-2",
"issue": 2206,
"summary": "landingPart answered the landing target from the roster alone, so a part id the shell's session list had not listed yet was handed to the session view. On such an id replaceMain's retain throws synchronously, and the 400 ms landing timer turned that into two uncaught errors on load. landingPart now returns only a destination the list knows when the caller passes it, in the same priority, and undefined when it knows none; landOn records refused and leaves the view untouched. The setPlain call moved below that guard, so a refused landing no longer clears the surface first.",
"files": [
"changelog.d/2206-landing-part-unknown.fixed.md",
"core/client.js",
"core/src/client/index.js",
"core/src/client/text.js",
"core/test/land-on.test.js",
"docs/subsystems/core-client.md",
"patches/ds-bot/0001-core-fixes.patch",
"patches/ds-bot/0003-flybot-api-client.patch",
"patches/ds-bot/0008-client-bundle.patch",
"patches/ds-bot/files/test/land-on.test.js"
],
"checks": {
"ran": "TZ=UTC ARCH_DOCS_BASE=77a6b4d4 npm run verify",
"result": "green",
"at": "ae42bd5b15c57ecb5445c69b74e5e15f27d89e3d"
},
"next": "none \u2014 draft; T1 waits for FlyBot Test's advisory PASS at this tip before ready"
}

…ab#2206)

The landing runs from a 400 ms timer with no click behind it, and the shell's
replaceMain calls sessions.retain, which throws synchronously on an id its
session list does not know (`sessions.retain: unknown session`). landingPart
answered with the roster alone, so an id the list had not listed yet — a cold
load, an archived part the roster still named as current — was handed to
openSession and became the two uncaught errors on load.

With the list in hand only a destination the list knows is returned, in the
same priority as before: a fresh part it has not caught up with falls back to
the chat it replaced, and with nothing known landingPart answers undefined and
landOn records `refused` and leaves the view where it is, instead of clearing
the surface for a navigation that will not happen. A caller that passes no list
keeps the id fallback.

Fixes ThinkFlowLab#2206
@HugoXK
HugoXK force-pushed the dev/2-landing-part-unknown branch from 50fba6f to ae42bd5 Compare November 2, 2026 17:43
@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Self-test: PASS at ae42bd5

Advisory report (diff only, not an acceptance gate), run by FlyBot Test and posted here by FlyBot Lead. It is not the repository reviewer's Verdict.

Coordinates read back from the API, not copied from the body: head ae42bd5…, tree 0c5be985b41d9f7f09ba434d5c0717ea91f008bd, single parent 77a6b4d… (= live main, re-read before and after), 1 commit, 10 files +92/−25, maintainer_can_modify true, fork ref HugoXK:dev/2-landing-part-unknown.

  1. Red first (clean git archive 77a6b4d4 tree carrying only the tip's core/test/land-on.test.js): # tests 7 / # pass 6 / # fail 1 — not ok 7 - #1306: a destination the session list does not know is never handed to the shell, "a session list that has not loaded: no destination", actual 'chief-2' vs expected undefined.

  2. At the tip: land-on.test.js + sidebar-readonly.test.js (the caller side of landOn) → 11/11; flybot-client.test.js → 14/14.

  3. Generators: node core/scripts/build-client.mjs --check exit 0 (silent). node scripts/ds-bot-patches.mjs check three ok lines (ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5); 10 patches apply cleanly in order; core/ equals ds-bot/ + patches/ds-bot/); canonical → generate is a no-op. Independent rebuild in a separate copy of the tip tree: sha256 be5e93ea755347f91e9c51630fcb233542e3e6fcf61ba9eb40676445dd7c577e, byte-identical to the committed core/client.js (cmp).

  4. Both deltas present in the bundle: core/client.js:2520 if (sessions === void 0) return destinations[0]; and :22435 if (target === void 0) return noteNavigation("refused", id, last?.sessionId);, with setPlain(false) after the guard (:22436). Patch side carries the same deltas (0001 text.js at :2758-2771, 0003 at :1661, 0008 at :1984/:24851); patches/ds-bot/files/test/land-on.test.js is diff -q identical to the core copy.

  5. Semantic probes, 8 green: fallback order four ways (friend-1 when the new part is unknown and the old chat is listed; chief-2 when the old chat is gone; undefined when nothing is known, with and without last; the known new part still wins); seven sessions === undefined cases byte-for-byte equal to the base behaviour; landOn guard lifted from the source bytes refuses an unknown target (openSession: [], setPlain: [], only a refused note) while the known target still navigates and clears, and the base-tree copy of the same guard does hand the id over and clear first — the differential the fix is about.

  6. TZ=UTC ARCH_DOCS_BASE=77a6b4d4… npm run verify exit 0: core 2665/2665, social 555/555 (0 skipped in this environment; the body's "554 + 1 skip" is environment-dependent, not a defect), tree 127, timeline 140, workflow 368, .github/scripts 598, interop 37, live-e2e 24, arch-docs: OK (10 changed file(s)), generate is a no-op, zero not ok lines anywhere.

@HugoXK
HugoXK marked this pull request as ready for review November 2, 2026 17:51
@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy review request — #2207 at ae42bd5

Ready for a verdict at this tip (Tier T1). Single commit, single parent 77a6b4d… (= live main), tree 0c5be985…, 10 files +92/−25, maintainer_can_modify: true, mergeable: clean.

Checks at this tip: all green (15 success + 1 cancelled agent-audience, non-blocking); tests-ok, merge-gate and enqueue-guard are success.

An advisory self-test at this exact tip is already on this PR (comment 6584214713, Self-test: PASS at ae42bd5b…, run by FlyBot Test and posted by FlyBot Lead): red-first 7/6/1 on a clean base tree, 11/11 and 14/14 at the tip, independent rebuild byte-identical (sha256 be5e93ea…), both bundle deltas present, eight semantic probes green including the base differential, verify exit 0 with arch-docs OK on 10 files. It is advisory, not a Verdict.

Nothing is requested of the reviewer beyond the verdict at this tip; the branch is HugoXK:dev/2-landing-part-unknown and maintainer edits are enabled.

@Birfy
Birfy added this pull request to the merge queue Nov 2, 2026
Merged via the queue into ThinkFlowLab:main with commit c64ba40 Nov 2, 2026
17 of 18 checks passed
@Birfy
Birfy deleted the dev/2-landing-part-unknown branch November 2, 2026 18:10
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.

fix(client): never hand the shell a session id its list does not know (uncaught sessions.retain on load)

2 participants